Skip to content

Commit

Permalink
Prevent CSV Injection (#210)
Browse files Browse the repository at this point in the history
* Add preventCsvInjection option

* Add tests for preventCsvInjection option

* Fix indentation problem that caused the build to fail

* Fix jsdoc asterix alignment that causes eslint to fail

* Add unit test for csv injectable characters when preventCsvInjection is not specified
  • Loading branch information
johnnyoshika committed Dec 16, 2021
1 parent b7615ce commit 1d7a947
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 1 deletion.
2 changes: 2 additions & 0 deletions README.md
Expand Up @@ -140,6 +140,8 @@ Looking for examples? Check out the Wiki: [json-2-csv Wiki](https://github.com/m
* Note: If selected, values will be converted using `toLocaleString()` rather than `toString()`
* `wrapBooleans` - Boolean - Should boolean values be wrapped in wrap delimiters to prevent Excel from converting them to Excel's TRUE/FALSE Boolean values.
* Default: `false`
* `preventCsvInjection` - Boolean - Should CSV injection be prevented by left trimming these characters: Equals (=), Plus (+), Minus (-), At (@), Tab (0x09), Carriage return (0x0D).
* Default: `false`


For examples, please refer to the [json2csv API Documentation (Link)](https://github.com/mrodrig/json-2-csv/wiki/json2csv-Documentation)
Expand Down
3 changes: 2 additions & 1 deletion lib/constants.json
Expand Up @@ -36,7 +36,8 @@
"useDateIso8601Format": false,
"useLocaleFormat": false,
"parseValue": null,
"wrapBooleans": false
"wrapBooleans": false,
"preventCsvInjection": false
},

"values" : {
Expand Down
7 changes: 7 additions & 0 deletions lib/converter.d.ts
Expand Up @@ -47,6 +47,13 @@ export interface ISharedOptions {
* @default false
*/
trimFieldValues?: boolean;

/**
* Should CSV injection be prevented by left trimming these characters:
* Equals (=), Plus (+), Minus (-), At (@), Tab (0x09), Carriage return (0x0D).
* @default false
*/
preventCsvInjection?: boolean;
}

export interface IFullOptions extends ISharedOptions {
Expand Down
21 changes: 21 additions & 0 deletions lib/json2csv.js
Expand Up @@ -252,6 +252,7 @@ const Json2Csv = function(options) {
processedRecordData = recordFieldData.map((fieldValue) => {
fieldValue = trimRecordFieldValue(fieldValue);
fieldValue = valueParserFn(fieldValue);
fieldValue = preventCsvInjection(fieldValue);
fieldValue = wrapFieldValueIfNecessary(fieldValue);

return fieldValue;
Expand Down Expand Up @@ -344,6 +345,26 @@ const Json2Csv = function(options) {
return fieldValue;
}

/**
* Prevent CSV injection on strings if specified by the user's provided options.
* Mitigation will be done by ensuring that the first character doesn't being with:
* Equals (=), Plus (+), Minus (-), At (@), Tab (0x09), Carriage return (0x0D).
* More info: https://owasp.org/www-community/attacks/CSV_Injection
* @param fieldValue
* @returns {*}
*/
function preventCsvInjection(fieldValue) {
if (options.preventCsvInjection) {
if (Array.isArray(fieldValue)) {
return fieldValue.map(preventCsvInjection);
} else if (utils.isString(fieldValue) && !utils.isNumber(fieldValue)) {
return fieldValue.replace(/^[=+\-@\t\r]+/g, '');
}
return fieldValue;
}
return fieldValue;
}

/**
* Escapes quotation marks in the field value, if necessary, and appropriately
* wraps the record field value if it contains a comma (field delimiter),
Expand Down
10 changes: 10 additions & 0 deletions lib/utils.js
Expand Up @@ -17,6 +17,7 @@ module.exports = {
getNCharacters,
unwind,
isInvalid,
isNumber,

// underscore replacements:
isString,
Expand Down Expand Up @@ -248,6 +249,15 @@ function unwind(array, field) {
return result;
}

/**
* Checks whether value can be converted to a number
* @param value {String}
* @returns {boolean}
*/
function isNumber(value) {
return !isNaN(Number(value));
}

/*
* Helper functions which were created to remove underscorejs from this package.
*/
Expand Down
118 changes: 118 additions & 0 deletions test/json2csv.js
Expand Up @@ -656,6 +656,124 @@ function runTests(jsonTestData, csvTestData) {
});
});

// Test cases for https://github.com/mrodrig/json-2-csv/issues/209
it('should left trim equals (=) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: '=Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim plus (+) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: '+Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim minus (-) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: '-Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim at (@) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: '@Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim tab (0x09) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: String.fromCharCode(9) + 'Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim carriage return (0x0D) if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: String.fromCharCode(13) + 'Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should left trim a combination of csv injection characters if preventCsvInjection is specified', (done) => {
converter.json2csv([{name: String.fromCharCode(9) + String.fromCharCode(13) + '=+-@Bob'}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'name\nBob';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should not alter numbers by removing minus (-) even if preventCsvInjection is specified', (done) => {
converter.json2csv([{temperature: -10}], (err, csv) => {
if (err) done(err);

let expectedCsv = 'temperature\n-10';

csv.should.equal(expectedCsv);
done();
}, {
preventCsvInjection: true
});
});

it('should not left trim a combination of csv injection characters if preventCsvInjection is not specified', (done) => {
let originalValue = String.fromCharCode(9) + String.fromCharCode(13) + '=+-@Bob';
converter.json2csv([{name: originalValue}], (err, csv) => {
if (err) done(err);

let expectedCsv = `name\n"${originalValue}"`;

csv.should.equal(expectedCsv);
done();
}, {
});
});

// Test case for #184
it('should handle keys with nested dots when expanding and unwinding arrays', (done) => {
converter.json2csv(jsonTestData.nestedDotKeysWithArrayExpandedUnwound, (err, csv) => {
Expand Down

0 comments on commit 1d7a947

Please sign in to comment.