From 436cc601389b5f3e5d60ef25ce8e5ce42745584b Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 17 Oct 2022 14:44:47 +0200 Subject: [PATCH] Ruby: update some uses of getConstantValue() --- ruby/ql/lib/codeql/ruby/Concepts.qll | 15 +++++++-------- .../codeql/ruby/frameworks/ActionController.qll | 4 ++-- .../lib/codeql/ruby/frameworks/ActiveRecord.qll | 4 +--- ruby/ql/lib/codeql/ruby/frameworks/Rails.qll | 7 +------ .../UnsafeDeserializationCustomizations.qll | 6 +----- ruby/ql/lib/codeql/ruby/security/XSS.qll | 5 ++--- .../security/cwe-078/NonConstantKernelOpen.ql | 2 +- .../library-tests/dataflow/summaries/Summaries.ql | 2 +- 8 files changed, 16 insertions(+), 29 deletions(-) diff --git a/ruby/ql/lib/codeql/ruby/Concepts.qll b/ruby/ql/lib/codeql/ruby/Concepts.qll index dd7c75565b7..031b64440bd 100644 --- a/ruby/ql/lib/codeql/ruby/Concepts.qll +++ b/ruby/ql/lib/codeql/ruby/Concepts.qll @@ -271,10 +271,7 @@ module Http { /** Gets the URL pattern for this route, if it can be statically determined. */ string getUrlPattern() { - exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode | - this.getUrlPatternArg().getALocalSource() = DataFlow::exprNode(strNode) and - result = strNode.getExpr().getConstantValue().getStringlikeValue() - ) + result = this.getUrlPatternArg().getALocalSource().getConstantValue().getStringlikeValue() } /** @@ -538,10 +535,12 @@ module Http { /** Gets the mimetype of this HTTP response, if it can be statically determined. */ string getMimetype() { - exists(CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode | - this.getMimetypeOrContentTypeArg().getALocalSource() = DataFlow::exprNode(strNode) and - result = strNode.getExpr().getConstantValue().getStringlikeValue().splitAt(";", 0) - ) + result = + this.getMimetypeOrContentTypeArg() + .getALocalSource() + .getConstantValue() + .getStringlikeValue() + .splitAt(";", 0) or not exists(this.getMimetypeOrContentTypeArg()) and result = this.getMimetypeDefault() diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll index e56df1465f6..1093d406ab8 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll @@ -234,7 +234,7 @@ private module Request { // Request headers are prefixed with `HTTP_` to distinguish them from // "headers" supplied by Rack middleware. this.getMethodName() = ["get_header", "fetch_header"] and - this.getArgument(0).asExpr().getExpr().getConstantValue().getString().regexpMatch("^HTTP_.+") + this.getArgument(0).getConstantValue().getString().regexpMatch("^HTTP_.+") } override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() } @@ -292,7 +292,7 @@ private module Request { EnvHttpAccess() { any(EnvCall c).(DataFlow::LocalSourceNode).flowsTo(this.getReceiver()) and this.getMethodName() = "[]" and - this.getArgument(0).asExpr().getExpr().getConstantValue().getString().regexpMatch("^HTTP_.+") + this.getArgument(0).getConstantValue().getString().regexpMatch("^HTTP_.+") } override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll b/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll index eff81adde23..e9109514af2 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll @@ -571,9 +571,7 @@ class ActiveRecordAssociation extends DataFlow::CallNode { * For example, in `has_many :posts`, this is `post`. */ string getTargetModelName() { - exists(string s | - s = this.getArgument(0).asExpr().getExpr().getConstantValue().getStringlikeValue() - | + exists(string s | s = this.getArgument(0).getConstantValue().getStringlikeValue() | // has_one :profile // belongs_to :user this.isSingular() and diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll b/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll index 97bd63453ee..74bd3c0a7e0 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Rails.qll @@ -212,12 +212,7 @@ private module Settings { private class LiteralSetting extends Setting { ConstantValue value; - LiteralSetting() { - exists(DataFlow::LocalSourceNode lsn | - lsn.asExpr().getConstantValue() = value and - lsn.flowsTo(this.getArgument(0)) - ) - } + LiteralSetting() { value = this.getArgument(0).getALocalSource().getConstantValue() } string getValueText() { result = value.toString() } diff --git a/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll b/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll index da759ea28e9..f12db1435d6 100644 --- a/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll +++ b/ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll @@ -88,11 +88,7 @@ module UnsafeDeserialization { private predicate isOjModePair(CfgNodes::ExprNodes::PairCfgNode p, string modeValue) { p.getKey().getConstantValue().isStringlikeValue("mode") and - exists(DataFlow::LocalSourceNode symbolLiteral, DataFlow::Node value | - symbolLiteral.asExpr().getExpr().getConstantValue().isSymbol(modeValue) and - symbolLiteral.flowsTo(value) and - value.asExpr() = p.getValue() - ) + DataFlow::exprNode(p.getValue()).getALocalSource().getConstantValue().isSymbol(modeValue) } /** diff --git a/ruby/ql/lib/codeql/ruby/security/XSS.qll b/ruby/ql/lib/codeql/ruby/security/XSS.qll index 7a3b4d2f0e7..8ab1d66446e 100644 --- a/ruby/ql/lib/codeql/ruby/security/XSS.qll +++ b/ruby/ql/lib/codeql/ruby/security/XSS.qll @@ -180,11 +180,10 @@ private module Shared { private predicate isFlowFromLocals0( CfgNodes::ExprNodes::ElementReferenceCfgNode refNode, string hashKey, ErbFile erb ) { - exists(DataFlow::Node argNode, CfgNodes::ExprNodes::StringlikeLiteralCfgNode strNode | + exists(DataFlow::Node argNode | argNode.asExpr() = refNode.getArgument(0) and refNode.getReceiver().getExpr().(MethodCall).getMethodName() = "local_assigns" and - argNode.getALocalSource() = DataFlow::exprNode(strNode) and - strNode.getExpr().getConstantValue().isStringlikeValue(hashKey) and + argNode.getALocalSource().getConstantValue().isStringlikeValue(hashKey) and erb = refNode.getFile() ) } diff --git a/ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.ql b/ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.ql index da490fd9ae3..a7cb2500c54 100644 --- a/ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.ql +++ b/ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.ql @@ -20,7 +20,7 @@ import codeql.ruby.ast.Literal from AmbiguousPathCall call where // there is not a constant string argument - not exists(call.getPathArgument().asExpr().getExpr().getConstantValue()) and + not exists(call.getPathArgument().getConstantValue()) and // if it's a format string, then the first argument is not a constant string not call.getPathArgument().getALocalSource().asExpr().getExpr().(StringLiteral).getComponent(0) instanceof StringTextComponent diff --git a/ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql b/ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql index fbe8f0af9d4..416a8635deb 100644 --- a/ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql +++ b/ruby/ql/test/library-tests/dataflow/summaries/Summaries.ql @@ -112,7 +112,7 @@ private class TypeFromCodeQL extends ModelInput::TypeModel { override DataFlow::Node getASource(string package, string type) { package = "test" and type = "FooOrBar" and - result.asExpr().getExpr().getConstantValue().getString() = "magic_string" + result.getConstantValue().getString() = "magic_string" } override API::Node getAnApiNode(string package, string type) {