-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Ruby: Add Unsafe HMAC Comparison Query. #13825
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some style/formatting comments, I haven't reviewed the results much yet but I'll do that next week.
} | ||
} | ||
|
||
class Configuration extends DataFlow::Configuration { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're moving away from extending DataFlow::Configuration
and instead towards implementing the DataFlow::ConfigSig
module. This would look something like:
private module UnsafeHmacComparison {
private module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
source instanceof OpenSslHmacHexdigest or
source instanceof OpenSslnewHmac or
source instanceof OpenSslHmacbase64digest or
source instanceof OpenSslHmacdigest or
source instanceof OpenSslHmactos
}
// Holds if a given sink is an Equality Operation (== or !=)
predicate isSink(DataFlow::Node sink) {
exists(EqualityOperation eqOp |
eqOp.getLeftOperand() = sink.asExpr().getExpr()
or
eqOp.getRightOperand() = sink.asExpr().getExpr()
)
}
}
import DataFlow::Global<Config>
}
and then
from UnsafeHmacComparison::PathNode source, UnsafeHmacComparison::PathNode sink
where UnsafeHmacComparison::flowPath(source, sink)
in the query itself.
There aren't that many Ruby examples yet, but rb/xpath-injection
uses this format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - will this pattern end up being deprecated at some point? We currently use this in one of our internal query packs.
@alexrford - I am getting a compilation error with this query. Could you give me a hand getting it resolved? Thanks. |
Co-authored-by: Alex Ford <[email protected]>
Co-authored-by: Alex Ford <[email protected]>
Hi CodeQL Team,
This adds an experimental query to detect potential Timing Attacks against usages of Ruby HMACs. This rule is partially inspired by the Python one.
The False positive rate seems fairly low on this query, but I did notice a large string of false positives in the Ruby repository after I ran this query against the top 1000 repositories in GitHub. Because I am unsure of why there are so many FP results in that repo, I figured it would be best to have this be an experimental query.