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

decodeEntities enables stored XSS against cveClient #962

Closed
ElectricNroff opened this issue Dec 19, 2022 · 1 comment
Closed

decodeEntities enables stored XSS against cveClient #962

ElectricNroff opened this issue Dec 19, 2022 · 1 comment

Comments

@ElectricNroff
Copy link
Contributor

Following up on the #729 issue, the new behavior enables XSS attacks against cveClient that were blocked in the CVE Services 2.1 release. In particular, regular users can conduct stored XSS attacks against admins (in their own organization) by using the API to change the name.first or name.last field. Common up-to-date web browsers are affected.

For example, suppose that bob@example.com is a regular user of the exampleCNA organization, and alice@example.com is an admin of that organization. Before Alice logs in at the https://certcc.github.io/cveClient/ website, Bob makes an API PUT request of

/org/exampleCNA/user/bob@example.com?name.first=%3cscript%3ealert%28%27JavaScript%20context:%20%27%2bdocument.domain.toUpperCase%28%29%29%3c%2fscript%3e&name.last=MyLastName

Then, when Alice logs in, the XSS payload executes in (at least) two places:

  • when clicking on Users (near the upper left)
  • when clicking on the line that has MyLastName in the Full name column

For example, in any recent version of Chrome, there is a popup window of:

certcc.github.io says
JavaScript context: CERTCC.GITHUB.IO

(Different XSS payloads would have more severe impacts, e.g., maybe exfiltrating Alice's API key to a server under Bob's control.)

This occurs because 7ad11ae makes decodeEntities calls that, in effect, undo the XSS protection provided by escape() in

query(['name.first']).optional().isString().trim().escape(),
query(['name.last']).optional().isString().trim().escape(),

Options include:

  1. revert some or all changes in 7ad11ae and re-open names of some users are stored with HTML entities #729 for possible future development work at a later time
  2. choose a different solution in which the response to API requests for user names (e.g., GET /org/{shortname}/users and GET /org/{shortname}/user/{username} won't ever have an XSS payload but can contain a single quote character (such as O'Reilly in the names of some users are stored with HTML entities #729 example)
  3. decide that this is not the fault of CVE Services, and inform authors of clients that it is their responsibility to handle malicious input in name fields (perhaps confirming that commonly used clients are fixed before the server behavior is changed)
  4. both 2 and 3

Note that, in the successful attack, Bob does not use the cveClient UI to change the name.first field. Instead, the attack scenario is that cveClient is used by the victim; the attacker can use any of various other clients (e.g., curl).

@jdaigneau5
Copy link
Collaborator

jdaigneau5 commented Nov 17, 2023

Removing decodeEntities() because it only changes the data in our DB and not for anyone who retrieves data through services. #1144 adds filters to prevent dangerous characters from being submitted, reducing the risk of XSS.

We'll be replacing express-validator's escape() with encodeURI because this will resolve issues with ' and &s.

encodeURI encodes more than we intended, so we'll rely on manually blocking certain characters with middleware instead.

jdaigneau5 added a commit that referenced this issue Nov 21, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…es a different set of characters. Also removed "decodeEntities"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants