Skip to content

Ruby: some improvements #10559

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Oct 4, 2022
Merged

Ruby: some improvements #10559

merged 12 commits into from
Oct 4, 2022

Conversation

aibaars
Copy link
Contributor

@aibaars aibaars commented Sep 23, 2022

I investigated a case where we didn't flag up a result for the rb/overly-permissive-file query .
This pull request implements the improvements needed to flag up that result, and makes the following changes:

  • track Pathname instances using global flow instead of local flow
  • model Object.dup and Kernel.tap
  • add flow steps for logical operators
  • add support for extend in the call graph

The dataflow improvements caused a fairly large number of false positives on our test suite for the missing regex anchor and incomplete hostname regex queries. This was caused by misclassifying some string literals as regular expression due to confusing Regexp#match and String#match. Commit f93bc90 improves this and remove the false positive results.

@github-actions github-actions bot added the Ruby label Sep 23, 2022
@aibaars aibaars force-pushed the cve-2019-3881 branch 2 times, most recently from d34d30f to fffa052 Compare September 26, 2022 10:58
@aibaars aibaars force-pushed the cve-2019-3881 branch 4 times, most recently from 9648d1a to 34fc1cb Compare September 28, 2022 15:02
@aibaars aibaars marked this pull request as ready for review October 3, 2022 08:29
@aibaars aibaars requested a review from a team as a code owner October 3, 2022 08:29
@aibaars aibaars added the no-change-note-required This PR does not need a change note label Oct 3, 2022
@hvitved hvitved force-pushed the cve-2019-3881 branch 2 times, most recently from 940e48e to 2d9fa98 Compare October 3, 2022 17:54
aibaars and others added 9 commits October 4, 2022 12:58
…'extend' calls

This avoids non-linear recursion at the cost of losing some results.
There are two flavours of `match?`. If the receiver of `match?` has type String
then the argument to `match?` is a regular expression. However, if the receiver of
`match?` has type Regexp then the argument is the text.

The role of receiver and argument flips depending on the type of the receiver, this
caused a lot of false positives when looking for string-like literals that are
used as a regular expression.

This commit attempts to improve things by trying to determine whether the type of the
receiver is known to be of type Regexp. In such cases we know that the argument
is unlikely to be  regular expression.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-change-note-required This PR does not need a change note Ruby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants