Skip to content

Commit 77b0c7f

Browse files
erik-krogherik-krogh
authored
Merge pull request #15221 from erik-krogh/react-step
JS: promote `PropsTaintStep` to a `PreCallGraphStep`
2 parents d6082f8 + 3000b4b commit 77b0c7f

File tree

5 files changed

+68
-4
lines changed

5 files changed

+68
-4
lines changed

‎javascript/ql/lib/semmle/javascript/frameworks/React.qll

+4-4
Original file line numberDiff line numberDiff line change
@@ -852,13 +852,13 @@ private class StateTaintStep extends TaintTracking::SharedTaintStep {
852852
}
853853

854854
/**
855-
* A taint propagating data flow edge for assignments of the form `c1.props.p = v`,
855+
* A data propagating data flow edge for assignments of the form `c1.props.p = v`,
856856
* where `c1` is an instance of React component `C`; in this case, we consider
857-
* taint to flow from `v` to any read of `c2.props.p`, where `c2`
857+
* data to flow from `v` to any read of `c2.props.p`, where `c2`
858858
* also is an instance of `C`.
859859
*/
860-
private class PropsTaintStep extends TaintTracking::SharedTaintStep {
861-
override predicate viewComponentStep(DataFlow::Node pred, DataFlow::Node succ) {
860+
private class PropsFlowStep extends PreCallGraphStep {
861+
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
862862
exists(ReactComponent c, string name, DataFlow::PropRead prn |
863863
prn = c.getAPropRead(name) or
864864
prn = c.getAPreviousPropsSource().getAPropertyRead(name)

‎javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected

+1
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@
7070
| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:92:14:92:16 | c.x |
7171
| getters-and-setters.js:79:20:79:27 | source() | getters-and-setters.js:100:10:100:22 | getX(new C()) |
7272
| getters-and-setters.js:89:17:89:24 | source() | getters-and-setters.js:82:18:82:22 | value |
73+
| importedReactComponent.jsx:4:40:4:47 | source() | exportedReactComponent.jsx:2:10:2:19 | props.text |
7374
| indexOf.js:4:11:4:18 | source() | indexOf.js:9:10:9:10 | x |
7475
| indexOf.js:4:11:4:18 | source() | indexOf.js:13:10:13:10 | x |
7576
| nested-props.js:4:13:4:20 | source() | nested-props.js:5:10:5:14 | obj.x |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/Xss.expected

+27
Original file line numberDiff line numberDiff line change
@@ -706,6 +706,18 @@ nodes
706706
| tooltip.jsx:11:25:11:30 | source |
707707
| tooltip.jsx:11:25:11:30 | source |
708708
| tooltip.jsx:11:25:11:30 | source |
709+
| tooltip.jsx:18:51:18:59 | provide() |
710+
| tooltip.jsx:18:51:18:59 | provide() |
711+
| tooltip.jsx:18:51:18:59 | provide() |
712+
| tooltip.jsx:18:51:18:59 | provide() |
713+
| tooltip.jsx:18:51:18:59 | provide() |
714+
| tooltip.jsx:22:11:22:30 | source |
715+
| tooltip.jsx:22:11:22:30 | source |
716+
| tooltip.jsx:22:20:22:30 | window.name |
717+
| tooltip.jsx:22:20:22:30 | window.name |
718+
| tooltip.jsx:22:20:22:30 | window.name |
719+
| tooltip.jsx:23:38:23:43 | source |
720+
| tooltip.jsx:23:38:23:43 | source |
709721
| translate.js:6:7:6:39 | target |
710722
| translate.js:6:16:6:39 | documen ... .search |
711723
| translate.js:6:16:6:39 | documen ... .search |
@@ -1882,6 +1894,20 @@ edges
18821894
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
18831895
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
18841896
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
1897+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1898+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1899+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1900+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1901+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:23:38:23:43 | source |
1902+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:23:38:23:43 | source |
1903+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1904+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1905+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1906+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1907+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1908+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1909+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1910+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
18851911
| translate.js:6:7:6:39 | target | translate.js:7:42:7:47 | target |
18861912
| translate.js:6:16:6:39 | documen ... .search | translate.js:6:7:6:39 | target |
18871913
| translate.js:6:16:6:39 | documen ... .search | translate.js:6:7:6:39 | target |
@@ -2486,6 +2512,7 @@ edges
24862512
| string-manipulations.js:10:16:10:45 | String( ... n.href) | string-manipulations.js:10:23:10:44 | documen ... on.href | string-manipulations.js:10:16:10:45 | String( ... n.href) | Cross-site scripting vulnerability due to $@. | string-manipulations.js:10:23:10:44 | documen ... on.href | user-provided value |
24872513
| tooltip.jsx:10:25:10:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:10:25:10:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
24882514
| tooltip.jsx:11:25:11:30 | source | tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:11:25:11:30 | source | Cross-site scripting vulnerability due to $@. | tooltip.jsx:6:20:6:30 | window.name | user-provided value |
2515+
| tooltip.jsx:18:51:18:59 | provide() | tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:18:51:18:59 | provide() | Cross-site scripting vulnerability due to $@. | tooltip.jsx:22:20:22:30 | window.name | user-provided value |
24892516
| translate.js:9:27:9:50 | searchP ... 'term') | translate.js:6:16:6:39 | documen ... .search | translate.js:9:27:9:50 | searchP ... 'term') | Cross-site scripting vulnerability due to $@. | translate.js:6:16:6:39 | documen ... .search | user-provided value |
24902517
| trusted-types-lib.js:2:12:2:12 | x | trusted-types.js:13:20:13:30 | window.name | trusted-types-lib.js:2:12:2:12 | x | Cross-site scripting vulnerability due to $@. | trusted-types.js:13:20:13:30 | window.name | user-provided value |
24912518
| trusted-types.js:3:67:3:67 | x | trusted-types.js:4:20:4:30 | window.name | trusted-types.js:3:67:3:67 | x | Cross-site scripting vulnerability due to $@. | trusted-types.js:4:20:4:30 | window.name | user-provided value |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/XssWithAdditionalSources.expected

+26
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,18 @@ nodes
718718
| tooltip.jsx:11:25:11:30 | source |
719719
| tooltip.jsx:11:25:11:30 | source |
720720
| tooltip.jsx:11:25:11:30 | source |
721+
| tooltip.jsx:18:51:18:59 | provide() |
722+
| tooltip.jsx:18:51:18:59 | provide() |
723+
| tooltip.jsx:18:51:18:59 | provide() |
724+
| tooltip.jsx:18:51:18:59 | provide() |
725+
| tooltip.jsx:18:51:18:59 | provide() |
726+
| tooltip.jsx:22:11:22:30 | source |
727+
| tooltip.jsx:22:11:22:30 | source |
728+
| tooltip.jsx:22:20:22:30 | window.name |
729+
| tooltip.jsx:22:20:22:30 | window.name |
730+
| tooltip.jsx:22:20:22:30 | window.name |
731+
| tooltip.jsx:23:38:23:43 | source |
732+
| tooltip.jsx:23:38:23:43 | source |
721733
| translate.js:6:7:6:39 | target |
722734
| translate.js:6:16:6:39 | documen ... .search |
723735
| translate.js:6:16:6:39 | documen ... .search |
@@ -1944,6 +1956,20 @@ edges
19441956
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
19451957
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
19461958
| tooltip.jsx:6:20:6:30 | window.name | tooltip.jsx:6:11:6:30 | source |
1959+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1960+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1961+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1962+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:18:51:18:59 | provide() |
1963+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:23:38:23:43 | source |
1964+
| tooltip.jsx:22:11:22:30 | source | tooltip.jsx:23:38:23:43 | source |
1965+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1966+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1967+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1968+
| tooltip.jsx:22:20:22:30 | window.name | tooltip.jsx:22:11:22:30 | source |
1969+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1970+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1971+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
1972+
| tooltip.jsx:23:38:23:43 | source | tooltip.jsx:18:51:18:59 | provide() |
19471973
| translate.js:6:7:6:39 | target | translate.js:7:42:7:47 | target |
19481974
| translate.js:6:16:6:39 | documen ... .search | translate.js:6:7:6:39 | target |
19491975
| translate.js:6:16:6:39 | documen ... .search | translate.js:6:7:6:39 | target |

‎javascript/ql/test/query-tests/Security/CWE-079/DomBasedXss/tooltip.jsx

+10
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,14 @@ function tooltips() {
1111
<span data-tip={source} data-html={true} /> // NOT OK
1212
<ReactTooltip />
1313
</span>
14+
}
15+
16+
function MyElement(props) {
17+
const provide = props.provide;
18+
return <div dangerouslySetInnerHTML={{__html: provide()}} />; // NOT OK
19+
}
20+
21+
function useMyElement() {
22+
const source = window.name;
23+
return <MyElement provide={() => source} />;
1424
}

0 commit comments

Comments
 (0)