Skip to content

Ruby: Reduce size of isLocalSourceNode #10089

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 1 commit into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,7 @@ private class Argument extends CfgNodes::ExprCfgNode {
cached
private module Cached {
private import TaintTrackingPrivate as TaintTrackingPrivate
private import codeql.ruby.typetracking.TypeTrackerSpecific as TypeTrackerSpecific

cached
newtype TNode =
Expand Down Expand Up @@ -332,21 +333,22 @@ private module Cached {

cached
predicate isLocalSourceNode(Node n) {
not n instanceof SynthHashSplatParameterNode and
(
n instanceof ParameterNode
or
n instanceof PostUpdateNodes::ExprPostUpdateNode
or
// Nodes that can't be reached from another entry definition or expression.
not reachedFromExprOrEntrySsaDef(n)
or
// Ensure all entry SSA definitions are local sources -- for parameters, this
// is needed by type tracking. Note that when the parameter has a default value,
// it will be reachable from an expression (the default value) and therefore
// won't be caught by the rule above.
entrySsaDefinition(n)
)
n instanceof ParameterNode and
not n instanceof SynthHashSplatParameterNode
or
// Expressions that can't be reached from another entry definition or expression
n instanceof ExprNode and
not reachedFromExprOrEntrySsaDef(n)
or
// Ensure all entry SSA definitions are local sources -- for parameters, this
// is needed by type tracking
entrySsaDefinition(n)
or
// Needed for flow out in type tracking
n instanceof SynthReturnNode
or
// Needed for stores in type tracking
TypeTrackerSpecific::basicStoreStep(_, n, _)
}

cached
Expand Down
30 changes: 14 additions & 16 deletions ruby/ql/src/queries/security/cwe-116/IncompleteSanitization.ql
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,10 @@ predicate allBackslashesEscaped(DataFlow::Node node) {
allBackslashesEscaped(node.getAPredecessor())
or
// general data flow from a (destructive) [g]sub!
exists(DataFlow::PostUpdateNode post, StringSubstitutionCall sub |
exists(StringSubstitutionCall sub |
sub.isDestructive() and
allBackslashesEscaped(sub) and
post.getPreUpdateNode() = sub.getReceiver() and
post.getASuccessor() = node
node.(DataFlow::PostUpdateNode).getPreUpdateNode() = sub.getReceiver()
)
}

Expand All @@ -125,19 +124,18 @@ predicate removesFirstOccurrence(StringSubstitutionCall sub, string str) {
* call.
*/
DataFlow::CallNode getAMethodCall(StringSubstitutionCall call) {
exists(DataFlow::Node receiver |
receiver = result.getReceiver() and
(
// for a non-destructive string substitution, is there flow from it to the
// receiver of another method call?
not call.isDestructive() and call.(DataFlow::LocalSourceNode).flowsTo(receiver)
or
// for a destructive string substitution, is there flow from its
// post-update receiver to the receiver of another method call?
call.isDestructive() and
exists(DataFlow::PostUpdateNode post | post.getPreUpdateNode() = call.getReceiver() |
post.(DataFlow::LocalSourceNode).flowsTo(receiver)
)
exists(DataFlow::Node receiver | receiver = result.getReceiver() |
// for a non-destructive string substitution, is there flow from it to the
// receiver of another method call?
not call.isDestructive() and
DataFlow::localFlow(call, receiver)
or
// for a destructive string substitution, is there flow from its
// post-update receiver to the receiver of another method call?
call.isDestructive() and
exists(DataFlow::PostUpdateNode post |
post.getPreUpdateNode() = call.getReceiver() and
DataFlow::localFlowStep+(post, receiver)
)
)
}
Expand Down
Loading