[False negative] Insufficient identification of React router location prop
- Lenguaje dominante
- CodeQL
- Estrellas
- 10.1k
- Forks
- 2.1k
- Merge medio
- 2 d 15 h
- PR fusionados (30 d)
- 141
Descripción
**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?
Guía de contribución
Evaluación
Este issue todavía no se ha evaluado.