github / github/codeql

[False negative] Insufficient identification of React router location prop

Abierto
#5,592 2 comentarios 0 reacciones 0 asignados Ver en GitHub
acknowledged enhancement JS
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

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.