Skip to content

JS: Sanitizer for sanitizer(x) === true #11769

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 4 commits into from
Feb 27, 2023
Merged

Conversation

erik-krogh
Copy link
Contributor

@erik-krogh erik-krogh commented Dec 21, 2022

Fixes #11667

The appliesTo change didn't change the result of any tests, but that feels like a lucky coincidence (because the nodes were instanceof DataFlow::BarrierGuardNode anyway).

Evaluations were uneventful:

@github-actions github-actions bot added the JS label Dec 21, 2022
@erik-krogh erik-krogh added the no-change-note-required This PR does not need a change note label Dec 21, 2022
@erik-krogh erik-krogh force-pushed the moreSan branch 2 times, most recently from dd0eb73 to 24a0836 Compare December 22, 2022 08:52
@erik-krogh erik-krogh marked this pull request as ready for review December 22, 2022 11:59
@erik-krogh erik-krogh requested a review from a team as a code owner December 22, 2022 11:59
class BooleanLiteral extends @boolean_literal, Literal { }
class BooleanLiteral extends @boolean_literal, Literal {
/** Gets the value of this literal. */
boolean getBoolValue() { if this.getRawValue() = "true" then result = true else result = false }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe consider naming consistency with TypeInference.qll:

  /** Gets the unique Boolean value that this node evaluates to, if any. */
  boolean getTheBooleanValue() { ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

getBoolValue is consistent with the predicate in YamlBool (In YAML.qll).
So I prefer to keep it as is.

exists(EqualityTest test, BooleanLiteral bool |
this.asExpr() = test and
test.hasOperands(prev.asExpr(), bool) and
polarity = test.getPolarity().booleanXor(bool.getBoolValue())
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure I understand the use of xor here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need polarity to flip both if test flips and if bool flips.
xor always flips when one of it's inputs flip, that's why I'm using it.

Then I had to figure out whether I had to add a .booleanNot() to the output or not, and that was just done by testing.

@aibaars
Copy link
Contributor

aibaars commented Feb 14, 2023

There are some merge conflicts.

Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
JS no-change-note-required This PR does not need a change note
Projects
None yet
Development

Successfully merging this pull request may close these issues.

False positive: About Javascript TaintBarriers
4 participants