Skip to content

Conversation

@RasmusWL
Copy link
Member

No description provided.

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.

Nice, shows the strength of previous modelling. Only minor nitpicks.

// hashing part
exists(StrConst str, DataFlow::Node hashNameArg |
hashNameArg in [this.getArg(2), this.getArgByName("hash_method")] and
DataFlow::exprNode(str).(DataFlow::LocalSourceNode).flowsTo(hashNameArg) and
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
DataFlow::exprNode(str).(DataFlow::LocalSourceNode).flowsTo(hashNameArg) and
DataFlow::exprNode(str) = hashNameArg.getALocalSource() and

I slightly prefer this form, as subtly implied in the documentation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I think taus recently did these rewrites as well. I obviously just copied from the old code 😄

@RasmusWL RasmusWL requested a review from yoff June 23, 2021 11:37
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

@codeql-ci codeql-ci merged commit 469e709 into github:main Jun 23, 2021
@RasmusWL RasmusWL deleted the rsa-modeling branch June 23, 2021 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants