type confusion for createUser authority.active_roles

[ElectricNroff]

Summary: this reports two similar anomalies that may be good to fix before a CVE Services 2.x release, but a fix isn't necessarily required then. A possible fix approach would be to ensure that the customSanitizer is operating on the expected data type. In one case, the code expects that the data type is a string but it's actually an object. In the other case, the code expects that the data type is a string but it's actually an array. The desired behavior in both cases is to produce an error message similar to {"error":"BAD_INPUT","message":"Parameters were invalid", ... instead.

Request

curl -X POST \
-H "CVE-API-ORG: myCNA" \
-H "CVE-API-USER: [email protected]" \
-H "CVE-API-KEY: ..." \
-d 'username=bob&authority.active_roles[a]=b' \
http://127.1:3000/api/org/myCNA/user

Value of err.stack in errorHandler in src/middleware/middleware.js

{"message":"TypeError: val.map is not a function
at Sanitization.sanitizer (/app/src/controller/org.controller/index.js:64:83)
at runCustomSanitizer (/app/node_modules/express-validator/src/context-items/sanitization.js:14:41)
at Sanitization.run (/app/node_modules/express-validator/src/context-items/sanitization.js:18:21)
at /app/node_modules/express-validator/src/chain/context-runner-impl.js:36:39
at Array.map (<anonymous>)
at ContextRunnerImpl.run (/app/node_modules/express-validator/src/chain/context-runner-impl.js:29:70)
at middleware (/app/node_modules/express-validator/src/middlewares/check.js:15:26)
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at next (/app/node_modules/express/lib/router/route.js:137:13)
at middleware (/app/node_modules/express-validator/src/middlewares/check.js:16:13)"}

Request

curl -X POST \
-H "CVE-API-ORG: myCNA" \
-H "CVE-API-USER: [email protected]" \
-H "CVE-API-KEY: ..." \
-d 'username=bob&authority.active_roles[][a]=b' \
http://127.1:3000/api/org/myCNA/user

Value of err.stack in errorHandler in src/middleware/middleware.js

{"message":"TypeError: x.toUpperCase is not a function
at /app/src/controller/org.controller/index.js:64:103
at Array.map (<anonymous>)
at Sanitization.sanitizer (/app/src/controller/org.controller/index.js:64:83)
at runCustomSanitizer (/app/node_modules/express-validator/src/context-items/sanitization.js:14:41)
at Sanitization.run (/app/node_modules/express-validator/src/context-items/sanitization.js:18:21)
at /app/node_modules/express-validator/src/chain/context-runner-impl.js:36:39
at Array.map (<anonymous>)
at ContextRunnerImpl.run (/app/node_modules/express-validator/src/chain/context-runner-impl.js:29:70)
at middleware (/app/node_modules/express-validator/src/middlewares/check.js:15:26)
at Layer.handle [as handle_request] (/app/node_modules/express/lib/router/layer.js:95:5)
at next (/app/node_modules/express/lib/router/route.js:137:13)
at middleware (/app/node_modules/express-validator/src/middlewares/check.js:16:13)"}

One concern is that the client user simply sees a 500 Server Error response, which may make it difficult to correct errors in a client application. Another concern is that these attempts to call nonexistent functions could perhaps contribute to a performance problem or security problem (but this is not proven to be possible).

cve-services/src/controller/org.controller/index.js

Line 64 in b10757d

body(['authority.active_roles']).optional().customSanitizer(val => { return val.map(x => { return x.toUpperCase() }) }).custom(val => { return isUserRole(val) }),

has val.map and x.toUpperCase calls that aren't guaranteed to be valid, because the client is allowed to control the data type.

This functionality is available via application/x-www-form-urlencoded data because the "extended syntax" of https://www.npmjs.com/package/qs is enabled by

cve-services/src/index.js

Line 35 in b10757d

app.use(express.urlencoded({ extended: true })) // Allows us to handle url encoded dat

[slubar]

Using an different format value (such as a string) from the expected array of strings for authority causes "service not available" error message. Update to give a more helpful error message

[slubar]

Related to #886 and #653