Ruby: make hex-escaped strings ("\xCD\xEF" etc.) sources of hardcoded data

Author: nickrolfe

ruby/ql/lib/codeql/ruby/security/HardcodedDataInterpretedAsCodeCustomizations.qll

@@ -7,6 +7,7 @@
 private import codeql.ruby.DataFlow
 private import codeql.ruby.security.CodeInjectionCustomizations
 private import codeql.ruby.AST as AST
+private import codeql.ruby.controlflow.CfgNodes
 
 /**
  * Provides default sources, sinks and sanitizers for reasoning about hard-coded
@@ -57,15 +58,32 @@ module HardcodedDataInterpretedAsCode {
    * least one digit), viewed as a source of hard-coded data that should not be
    * interpreted as code.
    */
-  private class DefaultSource extends Source, DataFlow::LocalSourceNode {
-    DefaultSource() {
-      exists(string val | this.asExpr().getConstantValue().isString(val) |
+  private class HexStringSource extends Source {
+    HexStringSource() {
+      exists(string val |
+        this.asExpr().(ExprNodes::StringLiteralCfgNode).getConstantValue().isString(val)
+      |
         val.regexpMatch("[0-9a-fA-F]{8,}") and
         val.regexpMatch(".*[0-9].*")
       )
     }
   }
 
+  /**
+   * A string literal whose raw text is made up entirely of `\x` escape
+   * sequences, viewed as a source of hard-coded data that should not be
+   * interpreted as code.
+   */
+  private class HexEscapedStringSource extends Source {
+    HexEscapedStringSource() {
+      forex(StringComponentCfgNode c |
+        c = this.asExpr().(ExprNodes::StringlikeLiteralCfgNode).getAComponent()
+      |
+        c.getNode().(AST::StringEscapeSequenceComponent).getRawText().prefix(2) = "\\x"
+      )
+    }
+  }
+
   /**
    * A code injection sink; hard-coded data should not flow here.
    */

ruby/ql/test/query-tests/security/cwe-506/HardcodedDataInterpretedAsCode.expected

@@ -6,6 +6,8 @@ edges
 | tst.rb:7:8:7:30 | totally_harmless_string :  | tst.rb:7:6:7:31 | call to e |
 | tst.rb:10:11:10:24 | "666f6f626172" :  | tst.rb:1:7:1:7 | r :  |
 | tst.rb:10:11:10:24 | "666f6f626172" :  | tst.rb:10:9:10:25 | call to e |
+| tst.rb:16:31:16:84 | "\\x70\\x75\\x74\\x73\\x28\\x27\\x68\\..." :  | tst.rb:17:6:17:32 | another_questionable_string :  |
+| tst.rb:17:6:17:32 | another_questionable_string :  | tst.rb:17:6:17:38 | call to strip |
 nodes
 | tst.rb:1:7:1:7 | r :  | semmle.label | r :  |
 | tst.rb:2:3:2:15 | call to pack :  | semmle.label | call to pack :  |
@@ -15,9 +17,13 @@ nodes
 | tst.rb:7:8:7:30 | totally_harmless_string :  | semmle.label | totally_harmless_string :  |
 | tst.rb:10:9:10:25 | call to e | semmle.label | call to e |
 | tst.rb:10:11:10:24 | "666f6f626172" :  | semmle.label | "666f6f626172" :  |
+| tst.rb:16:31:16:84 | "\\x70\\x75\\x74\\x73\\x28\\x27\\x68\\..." :  | semmle.label | "\\x70\\x75\\x74\\x73\\x28\\x27\\x68\\..." :  |
+| tst.rb:17:6:17:32 | another_questionable_string :  | semmle.label | another_questionable_string :  |
+| tst.rb:17:6:17:38 | call to strip | semmle.label | call to strip |
 subpaths
 | tst.rb:7:8:7:30 | totally_harmless_string :  | tst.rb:1:7:1:7 | r :  | tst.rb:2:3:2:15 | call to pack :  | tst.rb:7:6:7:31 | call to e |
 | tst.rb:10:11:10:24 | "666f6f626172" :  | tst.rb:1:7:1:7 | r :  | tst.rb:2:3:2:15 | call to pack :  | tst.rb:10:9:10:25 | call to e |
 #select
 | tst.rb:7:6:7:31 | call to e | tst.rb:5:27:5:72 | "707574732822636f646520696e6a6..." :  | tst.rb:7:6:7:31 | call to e | Hard-coded data from $@ is interpreted as code. | tst.rb:5:27:5:72 | "707574732822636f646520696e6a6..." | here |
 | tst.rb:10:9:10:25 | call to e | tst.rb:10:11:10:24 | "666f6f626172" :  | tst.rb:10:9:10:25 | call to e | Hard-coded data from $@ is interpreted as an import path. | tst.rb:10:11:10:24 | "666f6f626172" | here |
+| tst.rb:17:6:17:38 | call to strip | tst.rb:16:31:16:84 | "\\x70\\x75\\x74\\x73\\x28\\x27\\x68\\..." :  | tst.rb:17:6:17:38 | call to strip | Hard-coded data from $@ is interpreted as code. | tst.rb:16:31:16:84 | "\\x70\\x75\\x74\\x73\\x28\\x27\\x68\\..." | here |

ruby/ql/test/query-tests/security/cwe-506/tst.rb

@@ -11,4 +11,8 @@ def e(r)
 require '666f6f626172'    # OK: no taint step between source and sink
 
 x = 'deadbeef'
-require e(x) # OK: doesn't meet our criteria for being a source
+require e(x) # OK: doesn't meet our criteria for being a source
+
+another_questionable_string = "\x70\x75\x74\x73\x28\x27\x68\x65\x6C\x6C\x6F\x27\x29"
+eval(another_questionable_string.strip) # NOT OK: eval("puts('hello'")
+eval(another_questionable_string)       # OK: no taint step between source and sink