Merge pull request #1144 from CVEProject/dr-944

Resolves issue #944 - Better filtering for invalid characters in query parameters

src/controller/cve-id.controller/index.js

@@ -86,6 +86,7 @@ router.get('/cve-id',
   */
   mw.validateUser,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']) }),
+  query(['page', 'state', 'cve_id_year', 'time_reserved.lt', 'time_reserved.gt', 'time_modified.lt', 'time_modified.gt']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }),
   query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.ID_STATES),
   query(['cve_id_year']).optional().isNumeric().matches(/^[0-9]{4}$/),
@@ -177,6 +178,7 @@ router.post('/cve-id',
   mw.validateUser,
   mw.onlyCnas,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['amount', 'batch_type', 'short_name', 'cve_year']) }),
+  query(['amount', 'batch_type', 'short_name', 'cve_year']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['amount']).isInt(),
   query(['batch_type']).optional().isString().trim().escape().customSanitizer(val => { return val.toLowerCase() }),
   query(['short_name']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }),
@@ -340,6 +342,7 @@ router.put('/cve-id/:id',
   mw.onlyCnas,
   param(['id']).isString().matches(CONSTANTS.CVE_ID_REGEX),
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['state', 'org']) }),
+  query(['state', 'org']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(MODIFYTARGETS).withMessage(errorMsgs.ID_MODIFY_STATES),
   query(['org']).optional().isString().trim().escape(),
   parseError,

src/controller/cve.controller/index.js

@@ -157,6 +157,7 @@ router.get('/cve',
   mw.validateUser,
   mw.onlySecretariatOrBulkDownload,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']) }),
+  query(['page', 'time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }),
   query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT),
   query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT),
@@ -244,6 +245,7 @@ router.get('/cve_cursor',
   mw.validateUser,
   mw.onlySecretariatOrBulkDownload,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']) }),
+  query(['time_modified.lt', 'time_modified.gt', 'state', 'count_only', 'assigner_short_name', 'assigner', 'cna_modified', 'adp_short_name', 'next_page', 'previous_page', 'limit']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['time_modified.lt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT),
   query(['time_modified.gt']).optional().isString().trim().escape().customSanitizer(val => { return toDate(val) }).not().isEmpty().withMessage(errorMsgs.TIMESTAMP_FORMAT),
   query(['state']).optional().isString().trim().escape().customSanitizer(val => { return val.toUpperCase() }).isIn(CHOICES).withMessage(errorMsgs.CVE_FILTERED_STATES),

src/controller/org.controller/index.js

@@ -77,6 +77,7 @@ router.get('/org',
   mw.validateUser,
   mw.onlySecretariat,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['page']) }),
+  query(['page']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }),
   parseError,
   parseGetParams,
@@ -310,6 +311,7 @@ router.put('/org/:shortname',
   mw.validateUser,
   mw.onlySecretariat,
   query().custom((query) => { return mw.validateQueryParameterNames(query, ['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']) }),
+  query(['new_short_name', 'id_quota', 'name', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   param(['shortname']).isString().trim().escape().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }),
   query(['new_short_name']).optional().isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }),
   query(['id_quota']).optional().not().isArray().isInt({ min: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_min, max: CONSTANTS.MONGOOSE_VALIDATION.Org_policies_id_quota_max }).withMessage(errorMsgs.ID_QUOTA),
@@ -717,6 +719,8 @@ router.put('/org/:shortname/user/:username',
     return mw.validateQueryParameterNames(query, ['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle',
       'name.suffix', 'active_roles.add', 'active_roles.remove'])
   }),
+  query(['active', 'new_username', 'org_short_name', 'name.first', 'name.last', 'name.middle',
+    'name.suffix', 'active_roles.add', 'active_roles.remove']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   param(['shortname']).isString().trim().escape().notEmpty().isLength({ min: CONSTANTS.MIN_SHORTNAME_LENGTH, max: CONSTANTS.MAX_SHORTNAME_LENGTH }),
   param(['username']).isString().trim().escape().notEmpty().custom(isValidUsername),
   query(['active']).optional().isBoolean({ loose: true }),

src/controller/user.controller/index.js

@@ -75,6 +75,7 @@ router.get('/users',
   mw.validateUser,
   mw.onlySecretariat,
   query(['page']).optional().isInt({ min: CONSTANTS.PAGINATOR_PAGE }),
+  query(['page']).custom((val) => { return mw.containsNoInvalidCharacters(val) }),
   parseError,
   parseGetParams,
   controller.ALL_USERS)

src/middleware/middleware.js

@@ -439,6 +439,19 @@ function toUpperCaseArray (val) {
   return newArr
 }
 
+// Check for the invalid characters <, >, and "
+function containsNoInvalidCharacters (val) {
+  const invalidCharacterList = ['<', '>', '"']
+  if (val) {
+    for (const invalidCharacter of invalidCharacterList) {
+      if (val.includes(invalidCharacter)) {
+        throw new Error('contains invalid character: ' + invalidCharacter)
+      }
+    }
+  }
+  return true
+}
+
 module.exports = {
   setCacheControl,
   optionallyValidateUser,
@@ -457,5 +470,6 @@ module.exports = {
   validateJsonSyntax,
   rateLimiter: limiter,
   isFlatStringArray,
-  toUpperCaseArray
+  toUpperCaseArray,
+  containsNoInvalidCharacters
 }

test/unit-tests/middleware/checkForInvalidCharacters.js

@@ -0,0 +1,35 @@
+const chai = require('chai')
+const expect = chai.expect
+
+const { containsNoInvalidCharacters } = require('../../../src/middleware/middleware')
+
+describe('Testing Invalid Character checker', () => {
+  context('negative tests', () => {
+    it('Should fail when detecting a >', async () => {
+      try {
+        containsNoInvalidCharacters('sa>fe')
+      } catch (err) {
+        expect(err.message).to.contain('contains invalid character: >')
+      }
+    })
+    it('Should fail when detecting a <', async () => {
+      try {
+        containsNoInvalidCharacters('sa<fe')
+      } catch (err) {
+        expect(err.message).to.contain('contains invalid character: <')
+      }
+    })
+    it('Should fail when detecting a "', async () => {
+      try {
+        containsNoInvalidCharacters('sa"fe')
+      } catch (err) {
+        expect(err.message).to.contain('contains invalid character: "')
+      }
+    })
+  })
+  context('positive test', () => {
+    it('Should pass if no invalid characters', async () => {
+      containsNoInvalidCharacters('safe')
+    })
+  })
+})