Fix report and grant endpoints: bot verification, grant-id binding, HTML sanitization - #919
Open
1440000bytes wants to merge 8 commits into
Open
Conversation
|
@1440000bytes is attempting to deploy a commit to the OpenSats Team on Vercel. A member of the Team first needs to authorize it. |
Author
Proof of concept |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

The
/api/reportand/api/grantendpoints did not verify Cloudflare Turnstile, unlike/api/githuband/api/sendgrid./api/reportalso trusted theissue_numberandemailfrom the request body and rendered report content into the confirmation email without sanitization./api/grantmatched the submitted value as a substring of any issue title or body, so it could be used as a keyword search over the reports repo.Together this let an unauthenticated caller create comments on arbitrary issues in the reports repo under the server GitHub token, send mail from the verified sender with attacker controlled HTML, and enumerate grant metadata.
Changes
/api/reportand/api/grantonassertTurnstile, matching/api/githuband/api/sendgrid. Wire the existingTurnstileWidgetinto the grant validation form and the report preview step so the legitimate flows keep working.grant_idthrough a sharedfindGrantIssuehelper./api/reportno longer trusts a client suppliedissue_number, and an unknown grant id returns 404 with nothing written.findGrantIssueenforces this for both endpoints.sanitize-html, so raw HTML, scripts, event handlers, andjavascript:links are stripped.Note: The confirmation email is still sent to the address in the request body. There is no verified email of record to bind it to, so restricting or removing that send is a maintainer decision. The options are to drop the external confirmation email or to send only to an on file address. Closing enumeration fully would need a second factor such as a
grant_idand email match on record, or higher entropy grant ids.