[False negative] Insufficient identification of React router location prop
- 主要語言
- CodeQL
- 星號
- 10.1k
- 分支
- 2.1k
- 平均合併
- 2 天 15 小時
- 30 天內合併 PR
- 141
描述
**Description of the issue**
The QL library in [React.qll](https://github.com/github/codeql/blob/main/javascript/ql/src/semmle/javascript/frameworks/React.qll) implements some logic to infer whether a React component is using a `location` prop which is passed by the react-router and marks it as tainted. As far as I understand, the logic is based on whether `useLocation()` is used or if the component imports `react-router`. However, since neither of the above is necessary in order for a component to use the location prop passed by the router this may lead to cases where the dataflow into such components is missed.
As an example, consider and component which is routed and simply uses the location prop as `this.props.location`.
**Mitigation**
An additional way to detect whether a location prop is passed by the router is to detect whether the component analyzed is used in a `Route` JSX element. This provides a reliable way to detect whether the location prop is coming from the router.
The following is a quick-and-dirty example on such an implementation. It can definitely be improved to include more component types and maybe other heuristics for when the name of the component is not available.
```
predicate isComponentRouted(ES2015Component component) {
exists(JSXElement el |
el.getName() = "Route" and
el.getAttributeByName("component").getValue().toString() = component.getName()
)
}
class ImprovedLocationSource extends DOM::LocationSource::Range {
ImprovedLocationSource() {
exists(ES2015Component component |
isComponentRouted(component) and
this = component.getAPropRead("location")
)
}
}
```
Does something like the above make sense as an enhancement to the current implementation or am I missing some aspect of this that may result in increasing the false positive rate?
貢獻指南
評估
這個 Issue 還沒有評估資料。