github / github/codeql

[False negative] Insufficient identification of React router location prop

Đang mở
#5,592 2 bình luận 0 reaction 0 người được giao Xem trên GitHub
acknowledged enhancement JS
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

**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?

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.