Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unhelpful error message for a field name with an initial $ character #608

Closed
ElectricNroff opened this issue Mar 24, 2022 · 7 comments · Fixed by #1003
Closed

Unhelpful error message for a field name with an initial $ character #608

ElectricNroff opened this issue Mar 24, 2022 · 7 comments · Fixed by #1003

Comments

@ElectricNroff
Copy link
Contributor

ElectricNroff commented Mar 24, 2022

Summary: this reports an anomaly that may be good to fix before a CVE Services 2.x release, but a fix isn't necessarily required then. The fix approach would be to capture this MongoError somehow, and send the client an error report with a reasonable level of detail.

At the https://github.com/CVEProject/cve-services/tree/b10757d07104a0e8f54ae7c8f78796aeb0dbb12f commit, sending this:

{"cnaContainer": {"x_a": {"$b": "c"},
                   "affected": [{"product": "p","vendor": "v",
                     "versions": [
                        {
                            "version": "1.2",
                            "status": "affected"
                        }
                    ]}],"descriptions": [ {"lang": "en", "value":
                                           "v p before 1.2 is insecure."}],
                  "problemTypes": [{"descriptions": [{
                      "description": "insecurity", "lang": "en"}]}],
                  "providerMetadata": {
                      "orgId": "00000000-0000-4000-8000-000000000000"},
                  "references": [{"url": "https://example.com"}]}}

to a cna endpoint results in this in err.stack in errorHandler in src/middleware/middleware.js

{"message":"MongoError: The dollar ($) prefixed field '$b' in 'cve.containers.cna.x_a.$b' is not valid for storage.
at Function.create (/app/node_modules/mongodb/lib/core/error.js:57:12)
at toError (/app/node_modules/mongodb/lib/utils.js:123:22)
at /app/node_modules/mongodb/lib/operations/common_functions.js:384:39
at handler (/app/node_modules/mongodb/lib/core/sdam/topology.js:944:24)
at /app/node_modules/mongodb/lib/cmap/connection_pool.js:350:13
at handleOperationResult (/app/node_modules/mongodb/lib/core/sdam/server.js:558:5)
at MessageStream.messageHandler (/app/node_modules/mongodb/lib/cmap/connection.js:277:5)
at MessageStream.emit (events.js:400:28)
at processIncomingData (/app/node_modules/mongodb/lib/cmap/message_stream.js:144:12)
at MessageStream._write (/app/node_modules/mongodb/lib/cmap/message_stream.js:42:5)
at writeOrBuffer (internal/streams/writable.js:358:12)
at MessageStream.Writable.write (internal/streams/writable.js:303:10)
at Socket.ondata (internal/streams/readable.js:731:22)
at Socket.emit (events.js:400:28)
at addChunk (internal/streams/readable.js:293:12)
at readableAddChunk (internal/streams/readable.js:267:9)"}

The concern is that the client user probably won't know that the dollar sign character is what leads to the 500 Server Error response. Attempts to use an initial dollar sign character for a custom field name are entirely realistic: for example, various documentation about constructing JSON documents recommends $schema as a field name (see the https://github.com/CVEProject/cve-schema/issues/144 issue). DocumentDB does not support $schema - the only allowed names beginning with $ are $db, $id, and $ref.

@wizedkyle
Copy link
Contributor

@ElectricNroff I am testing this on the dev branch and can't replicate the problem. I did a POST request to /cve/{cve-id}/cna with the request body above and got a 200 response.

I'm not sure if I am testing this wrong or missing something would you be able to provide some more information on how to reproduce this issue please?

@jdaigneau5 jdaigneau5 assigned jdaigneau5 and unassigned jdaigneau5 Mar 30, 2022
@ElectricNroff
Copy link
Contributor Author

"testing this on the dev branch" is probably not the best way to describe a test environment, because the dev branch changes from time to time, and the behavior is specific to the database implementation.

# use a mongo image that most closely matches the DocumentDB API
image: mongo:3.6.20-xenial

says that mongo:3.6.20-xenial is used. The behavior is different with later MongoDB versions, as discussed in the https://www.mongodb.com/docs/manual/core/dot-dollar-considerations/ article (etc.). The production version of CVE Services is anticipated to use Amazon DocumentDB, not MongoDB. You may want to test against cveawg-test.mitre.org for this specific issue of a '$' in a field name. For example, reserving a new CVE ID and then posting the following to https://cveawg-test.mitre.org/api/cve/CVE-2022-####/cna

{"cnaContainer": {"x_a": {"$b": "c"},
                   "affected": [{"product": "p","vendor": "v",
                     "versions": [
                        {
                            "version": "1.2",
                            "status": "affected"
                        }
                    ]}],"descriptions": [ {"lang": "en", "value":
                                           "v p 1.2 is insecure."}],
                  "problemTypes": [{"descriptions": [{
                      "description": "insecurity", "lang": "en"}]}],
                  "providerMetadata": {
                      "orgId": "00000000-0000-4000-8000-000000000000"},
                  "references": [{"url": "https://example.com"}]}}

results in

HTTP/2 500

{"error":"SERVICE_NOT_AVAILABLE","message":"This service appears to not be available."}

Simply deleting the '$' and instead sending:

{"cnaContainer": {"x_a": {"b": "c"},
                   "affected": [{"product": "p","vendor": "v",
                     "versions": [
                        {
                            "version": "1.2",
                            "status": "affected"
                        }
                    ]}],"descriptions": [ {"lang": "en", "value":
                                           "v p 1.2 is insecure."}],
                  "problemTypes": [{"descriptions": [{
                      "description": "insecurity", "lang": "en"}]}],
                  "providerMetadata": {
                      "orgId": "00000000-0000-4000-8000-000000000000"},
                  "references": [{"url": "https://example.com"}]}}

results in:

HTTP/2 200

{"message":"CVE-2022-#### record was successfully created.", ...

The error handling isn't identical, but the principle is the same. A '$' in a field name will, most likely, not be supported in production, and the CVE Services code needs to produce a valid error report in this situation.

@slubar
Copy link
Contributor

slubar commented Apr 6, 2022

I have tried to reproduce this on the test instance, but only see
HTTP/2 500
{"error":"SERVICE_NOT_AVAILABLE","message":"This service appears to not be available."}

Which is not a helpful message, but does not expose the information reported in this issue. @ElectricNroff please comment with instructions on how to reproduce the Mongo error including on which endpoint it occurs and the specific parameters that you are using

@ElectricNroff ElectricNroff changed the title CWE-497 for a field name with an initial $ character Unhelpful error message for a field name with an initial $ character Apr 6, 2022
@ElectricNroff
Copy link
Contributor Author

I've modified this issue to reflect the "not a helpful message" angle, because this is the largest concern that can be reproduced with the current dev branch.

The endpoint can be /api/cve/CVE-2022-20001/cna for a POST or PUT request with no parameters.

To reproduce, follow the https://github.com/CVEProject/cve-services/blob/f5d656f479cbfdbdd26fb0acd4cd549cae71cb1a/docker/README.md process. It is important to ensure that the database is based on MongoDB 3.x. (Later MongoDB versions have more flexibility on $ characters, but the initial release of CVE Services 2.x may rely on a DocumentDB release that omits that flexibility.) Specifically, one can use these steps to check:

% docker-compose exec -u root docdb /bin/sh
# cat /etc/issue

This should show Ubuntu 16.04.7 LTS, which is another name for "xenial" as shown at

image: mongo:3.6.20-xenial

# apt list --installed | fgrep mongo
...
mongodb-org/now 3.6.20 amd64 [installed,local]
mongodb-org-mongos/now 3.6.20 amd64 [installed,local]
mongodb-org-server/now 3.6.20 amd64 [installed,local]
mongodb-org-shell/now 3.6.20 amd64 [installed,local]
mongodb-org-tools/now 3.6.20 amd64 [installed,local]

This shows that version 3.6.20 is being used.

The client will receive a 500 Server Error message. A possible solution approach is to add two levels of error checking. First, look for the MongoError: The dollar ($) prefixed field error, and then check for all other instances of MongoError.

There may be a few different reasonable design options. One approach is to simply insert the new checks at the beginning of validateJsonSyntax in src/middleware/middleware.js

  if (err.stack && err.stack.includes('MongoError: The dollar')) {
    console.warn('Request failed validation because JSON document has a dollar sign in a field name')
    console.info((JSON.stringify(err)))
    return res.status(400).json(error.fieldHasDollarSign())
  }
  if (err.stack && err.stack.includes('MongoError:')) {
    console.warn('Request failed validation because of an issue at the MongoDB or DocumentDB layer')
    console.info((JSON.stringify(err)))
    return res.status(400).json(error.genericMongoLayer())
  }

and then add these to the available error types in src/middleware/error.js

 fieldHasDollarSign () { // mw
    const err = {}
    err.error = 'BAD_REQUEST'
    err.message = 'JSON document must not have an arbitrary field name starting with a dollar sign.'
    return err
  }

 genericMongoLayer () { // mw
    const err = {}
    err.error = 'BAD_REQUEST'
    err.message = 'There was an unexpected issue at the MongoDB or DocumentDB layer.'
    return err
  }

Google searches suggest that DocumentDB, like MongoDB, produces error messages beginning with MongoError but I have not directly tested this.

If the change approach outlined above is used with MongoDB 3.6.20, then the client receives a 400 error status with more helpful details than before (the complete error stack is not sent to the client).

@slubar slubar added this to the CVE Services 2.1 milestone Oct 26, 2022
@slubar slubar added this to Needs Triage in Issue Triage via automation Oct 26, 2022
@slubar
Copy link
Contributor

slubar commented Oct 26, 2022

Related to #853

@jdaigneau5 jdaigneau5 moved this from Needs Triage to Low Priority in Issue Triage Oct 31, 2022
@slubar
Copy link
Contributor

slubar commented Jan 18, 2023

Update UNABLE_TO_STORE_CVE_RECORD error message to read something like, "A problem occurred while saving the CVE Record; ensure that x_ values do not start with $"

@slubar
Copy link
Contributor

slubar commented Jan 20, 2023

JSON 5.0 schema issue #209 has been opened in relation to this issue

@jdaigneau5 jdaigneau5 removed this from Low Priority in Issue Triage Jan 23, 2023
@jdaigneau5 jdaigneau5 moved this from To Do to In Progress in Sprint 24 January 23 - February 3 Jan 26, 2023
@jdaigneau5 jdaigneau5 self-assigned this Jan 26, 2023
@jdaigneau5 jdaigneau5 moved this from In Progress to In Review in Sprint 24 January 23 - February 3 Feb 1, 2023
slubar added a commit that referenced this issue Feb 1, 2023
Resolves #608 Improved error message for creating CVE records with $ in X_ values
Sprint 24 January 23 - February 3 automation moved this from In Review to Done Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
4 participants