From f7fc2e0b0076afb14af30a995ce208d2eaa8e72b Mon Sep 17 00:00:00 2001 From: Harry Maclean Date: Wed, 1 May 2024 16:13:39 +0100 Subject: [PATCH] Ruby: Fix StringSubstitutionCall charpred Some missing parens meant this class targeted way more things than intended. --- ruby/ql/lib/codeql/ruby/frameworks/core/String.qll | 8 +++++--- .../security/cwe-116/IncompleteSanitization/tst.rb | 5 +++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/frameworks/core/String.qll b/ruby/ql/lib/codeql/ruby/frameworks/core/String.qll index 86246ba80a20..23a902c00195 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/core/String.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/core/String.qll @@ -19,9 +19,11 @@ class StringSubstitutionCall extends DataFlow::CallNode { StringSubstitutionCall() { this.getMethodName() = ["sub", "sub!", "gsub", "gsub!"] and exists(this.getReceiver()) and - this.getNumberOfArguments() = 2 - or - this.getNumberOfArguments() = 1 and exists(this.getBlock()) + ( + this.getNumberOfArguments() = 2 + or + this.getNumberOfArguments() = 1 and exists(this.getBlock()) + ) } /** diff --git a/ruby/ql/test/query-tests/security/cwe-116/IncompleteSanitization/tst.rb b/ruby/ql/test/query-tests/security/cwe-116/IncompleteSanitization/tst.rb index 569e8b5ac09a..f59fdd332aed 100644 --- a/ruby/ql/test/query-tests/security/cwe-116/IncompleteSanitization/tst.rb +++ b/ruby/ql/test/query-tests/security/cwe-116/IncompleteSanitization/tst.rb @@ -268,3 +268,8 @@ def bad_path_sanitizer(p1, p2) p1.sub! "/../", "" # NOT OK p2.sub "/../", "" # NOT OK end + +def each_line_sanitizer(p1) + p1.each_line("\n") do |l| # OK - does no sanitization + end +end