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

JS: React-relay support #18858

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

JS: React-relay support #18858

wants to merge 6 commits into from

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Feb 25, 2025

This pull request introduces a library model for react-relay's, categorizing it's new sources as a response threat.
Closes #465

@Napalys Napalys force-pushed the js/react-relay branch 6 times, most recently from a107b89 to aea5fed Compare February 26, 2025 09:38
@Napalys Napalys marked this pull request as ready for review March 7, 2025 07:54
@Copilot Copilot bot review requested due to automatic review settings March 7, 2025 07:54
@Napalys Napalys requested a review from a team as a code owner March 7, 2025 07:54

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This pull request adds support for the react-relay library by introducing a new model that classifies specific react-relay hook return values as a "response" threat and extends test coverage for DOM-based XSS scenarios.

  • Introduces a YAML file to model response threats for various react-relay hooks
  • Adds corresponding test cases in a React component to validate DOM-based XSS alerts
  • Updates change notes to document the new support for react-relay

Reviewed Changes

File Description
javascript/ql/lib/ext/react-relay-threat.model.yml Adds model definitions for marking react-relay hook return values as "response" threats
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXssWithResponseThreat/testReactRelay.tsx Provides test cases for new response threat scenarios using react-relay hooks
javascript/ql/lib/change-notes/2025-02-25-react-relay.md Notes the addition of react-relay support

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Tip: Leave feedback on Copilot's review comments with the 👎 and 👍 buttons to help improve review quality. Learn more

@github github deleted a comment from Copilot bot Mar 7, 2025
@Napalys Napalys changed the title JS: React-relay useFragment support JS: React-relay support Mar 10, 2025
- ["react-relay", "Member[useLazyLoadQuery].ReturnValue", "response"]
- ["react-relay", "Member[usePreloadedQuery].ReturnValue", "response"]
- ["react-relay", "Member[useClientQuery].ReturnValue", "response"]
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"]
- ["react-relay", "Member[useRefetchableFragment].ReturnValue.Member[0]", "response"]

Based on the code sample it seems the data is returned in the first array element:

const [data, refetch] = useRefetchableFragment(

- ["react-relay", "Member[usePreloadedQuery].ReturnValue", "response"]
- ["react-relay", "Member[useClientQuery].ReturnValue", "response"]
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"]
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"]
- ["react-relay", "Member[usePaginationFragment].ReturnValue.Member[data]", "response"]

- ["react-relay", "Member[useClientQuery].ReturnValue", "response"]
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"]
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"]
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"]
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Parameter[0]", "response"]

For complicated reasons our implementation considers Argument and Parameter to be interchangeable in some places, but it's more appropriate to use Parameter here.

- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"]
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"]
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"]
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"]
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Parameter[0]", "response"]

- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"]
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"]
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"]
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Argument[0]", "response"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Argument[0]", "response"]
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Parameter[0]", "response"]

const [data, refetch] = useRefetchableFragment(query, props.comment); // $ Source
return (
<>
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a line here to ensure refetch isn't seen as a source.

Suggested change
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert
<h1 dangerouslySetInnerHTML={{ __html: refetch }} /> // OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants