Skip to content

ReDoS: convert RelevantState to a class in the PrefixConstruction module#10285

Merged
erik-krogh merged 4 commits intogithub:mainfrom
erik-krogh:paramClass
Sep 12, 2022
Merged

ReDoS: convert RelevantState to a class in the PrefixConstruction module#10285
erik-krogh merged 4 commits intogithub:mainfrom
erik-krogh:paramClass

Conversation

@erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Sep 2, 2022

I just learned that class X instanceof Y { inside a parameterized module is allowed.
So now I'm using it.

Evaluations were uneventful: Ruby, Java, Python, JavaScript.

@erik-krogh erik-krogh marked this pull request as ready for review September 5, 2022 07:29
@erik-krogh erik-krogh requested review from a team as code owners September 5, 2022 07:29
@aibaars aibaars added the no-change-note-required This PR does not need a change note label Sep 5, 2022
Copy link
Contributor

@aibaars aibaars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. One suggestion.

erik-krogh and others added 2 commits September 5, 2022 11:22
Co-authored-by: Arthur Baars <aibaars@github.com>
yoff
yoff previously approved these changes Sep 5, 2022
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sweet, this cleans up nicely :-)

/** A state within a regular expression that contains a candidate state. */
class RelevantState instanceof State {
RelevantState() {
exists(State s | isCandidate(s) | getRoot(s.getRepr()) = getRoot(this.getRepr()))

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

PredConsistency::multipleResolveCall
/** A state within a regular expression that contains a candidate state. */
class RelevantState instanceof State {
RelevantState() {
exists(State s | isCandidate(s) | getRoot(s.getRepr()) = getRoot(this.getRepr()))

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

PredConsistency::multipleResolveCall
/** A state within a regular expression that contains a candidate state. */
class RelevantState instanceof State {
RelevantState() {
exists(State s | isCandidate(s) | getRoot(s.getRepr()) = getRoot(this.getRepr()))

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

PredConsistency::multipleResolveCall
/** A state within a regular expression that contains a candidate state. */
class RelevantState instanceof State {
RelevantState() {
exists(State s | isCandidate(s) | getRoot(s.getRepr()) = getRoot(this.getRepr()))

Check warning

Code scanning / CodeQL

QL-for-QL encountered an internal consistency error

PredConsistency::multipleResolveCall
@erik-krogh
Copy link
Contributor Author

erik-krogh commented Sep 5, 2022

The QL-for-QL warnings can be ignored, they should be fixed by this PR: #10250

@erik-krogh erik-krogh requested a review from yoff September 9, 2022 06:35
@erik-krogh erik-krogh changed the title convert RelevantState to a class in the PrefixConstruction module ReDoS: convert RelevantState to a class in the PrefixConstruction module Sep 12, 2022
Copy link
Contributor

@yoff yoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@erik-krogh erik-krogh merged commit 818601b into github:main Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants