hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-29 18:55:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'main' into rb/fix-spurious-singleton-calls

Browse Source
This commit is contained in:
Asger F
2022-10-14 15:52:38 +02:00
parent 1bd3d29409 4dcf4f2d2c
commit 8cb4f230d8
74 changed files with 1254 additions and 294 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
docs/codeql/support/reusables/frameworks.rst
View File

@@ -221,11 +221,15 @@ and the CodeQL library pack ``codeql/python-all`` (`changelog <https://github.co
aiopg, Database
asyncpg, Database
clickhouse-driver, Database
cx_Oracle, Database
mysql-connector-python, Database
mysql-connector, Database
MySQL-python, Database
mysqlclient, Database
oracledb, Database
phoenixdb, Database
psycopg2, Database
pyodbc, Database
pymssql, Database
PyMySQL, Database
sqlite3, Database
@@ -241,3 +245,19 @@ and the CodeQL library pack ``codeql/python-all`` (`changelog <https://github.co
libxml2, XML processing library
lxml, XML processing library
xmltodict, XML processing library
Ruby built-in support
====================================
Provided by the current versions of the
CodeQL query pack ``codeql/ruby-queries`` (`changelog <https://github.com/github/codeql/tree/codeql-cli/latest/ruby/ql/src/CHANGELOG.md>`__, `source <https://github.com/github/codeql/tree/codeql-cli/latest/ruby/ql/src>`__)
and the CodeQL library pack ``codeql/ruby-all`` (`changelog <https://github.com/github/codeql/tree/codeql-cli/latest/ruby/ql/lib/CHANGELOG.md>`__, `source <https://github.com/github/codeql/tree/codeql-cli/latest/ruby/ql/lib>`__).
.. csv-table::
:header-rows: 1
:class: fullWidthTable
:widths: auto
Name, Category
Ruby on Rails, Web framework

2
docs/codeql/support/reusables/versions-compilers.rst
View File

@@ -22,7 +22,7 @@
Eclipse compiler for Java (ECJ) [5]_",``.java``
JavaScript,ECMAScript 2022 or lower,Not applicable,"``.js``, ``.jsx``, ``.mjs``, ``.es``, ``.es6``, ``.htm``, ``.html``, ``.xhtm``, ``.xhtml``, ``.vue``, ``.hbs``, ``.ejs``, ``.njk``, ``.json``, ``.yaml``, ``.yml``, ``.raml``, ``.xml`` [6]_"
Python [7]_,"2.7, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10",Not applicable,``.py``
Ruby [8]_,"up to 3.0.2",Not applicable,"``.rb``, ``.erb``, ``.gemspec``, ``Gemfile``"
Ruby [8]_,"up to 3.1",Not applicable,"``.rb``, ``.erb``, ``.gemspec``, ``Gemfile``"
TypeScript [9]_,"2.6-4.8",Standard TypeScript compiler,"``.ts``, ``.tsx``, ``.mts``, ``.cts``"
.. container:: footnote-group

8
java/kotlin-extractor/src/main/kotlin/KotlinExtractorExtension.kt
View File

@@ -74,7 +74,7 @@ class KotlinExtractorExtension(
// First, if we can find our log directory, then let's try
// making a log file there:
val extractorLogDir = System.getenv("CODEQL_EXTRACTOR_JAVA_LOG_DIR")
if (extractorLogDir != null || extractorLogDir != "") {
if (extractorLogDir != null && extractorLogDir != "") {
// We use a slightly different filename pattern compared
// to normal logs. Just the existence of a `-top` log is
// a sign that something's gone very wrong.
@@ -296,7 +296,9 @@ private fun doFile(
context.clear()
}
val dbSrcFilePath = Paths.get("$dbSrcDir/$srcFilePath")
val srcFileRelativePath = srcFilePath.replace(':', '_')
val dbSrcFilePath = Paths.get("$dbSrcDir/$srcFileRelativePath")
val dbSrcDirPath = dbSrcFilePath.parent
Files.createDirectories(dbSrcDirPath)
val srcTmpFile = File.createTempFile(dbSrcFilePath.fileName.toString() + ".", ".src.tmp", dbSrcDirPath.toFile())
@@ -305,7 +307,7 @@ private fun doFile(
}
srcTmpFile.renameTo(dbSrcFilePath.toFile())
val trapFileName = "$dbTrapDir/$srcFilePath.trap"
val trapFileName = "$dbTrapDir/$srcFileRelativePath.trap"
val trapFileWriter = getTrapFileWriter(compression, logger, trapFileName)
if (checkTrapIdentical || !trapFileWriter.exists()) {

4
java/ql/lib/change-notes/2022-10-13-stream-collect.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added support for common patterns involving `Stream.collect` and common collectors like `Collectors.toList()`.

6
java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll
View File

@@ -75,7 +75,7 @@ import java
private import semmle.code.java.dataflow.DataFlow::DataFlow
private import internal.DataFlowPrivate
private import internal.FlowSummaryImpl::Private::External
private import internal.FlowSummaryImplSpecific
private import internal.FlowSummaryImplSpecific as FlowSummaryImplSpecific
private import internal.AccessPathSyntax
private import FlowSummary
@@ -834,7 +834,7 @@ private module Cached {
*/
cached
predicate sourceNode(Node node, string kind) {
exists(InterpretNode n | isSourceNode(n, kind) and n.asNode() = node)
exists(FlowSummaryImplSpecific::InterpretNode n | isSourceNode(n, kind) and n.asNode() = node)
}
/**
@@ -843,7 +843,7 @@ private module Cached {
*/
cached
predicate sinkNode(Node node, string kind) {
exists(InterpretNode n | isSinkNode(n, kind) and n.asNode() = node)
exists(FlowSummaryImplSpecific::InterpretNode n | isSinkNode(n, kind) and n.asNode() = node)
}
}

126
java/ql/lib/semmle/code/java/dataflow/FlowSummary.qll
View File

@@ -4,7 +4,6 @@
import java
private import internal.FlowSummaryImpl as Impl
private import internal.DataFlowDispatch
private import internal.DataFlowUtil
// import all instances of SummarizedCallable below
@@ -24,6 +23,12 @@ module SummaryComponent {
/** Gets a summary component for field `f`. */
SummaryComponent field(Field f) { result = content(any(FieldContent c | c.getField() = f)) }
/** Gets a summary component for `Element`. */
SummaryComponent element() { result = content(any(CollectionContent c)) }
/** Gets a summary component for `MapValue`. */
SummaryComponent mapValue() { result = content(any(MapValueContent c)) }
/** Gets a summary component that represents the return value of a call. */
SummaryComponent return() { result = return(_) }
}
@@ -42,10 +47,129 @@ module SummaryComponentStack {
result = push(SummaryComponent::field(f), object)
}
/** Gets a stack representing `Element` of `object`. */
SummaryComponentStack elementOf(SummaryComponentStack object) {
result = push(SummaryComponent::element(), object)
}
/** Gets a stack representing `MapValue` of `object`. */
SummaryComponentStack mapValueOf(SummaryComponentStack object) {
result = push(SummaryComponent::mapValue(), object)
}
/** Gets a singleton stack representing a (normal) return. */
SummaryComponentStack return() { result = singleton(SummaryComponent::return()) }
}
/** A synthetic callable with a set of concrete call sites and a flow summary. */
abstract class SyntheticCallable extends string {
bindingset[this]
SyntheticCallable() { any() }
/** Gets a call that targets this callable. */
abstract Call getACall();
/**
* Holds if data may flow from `input` to `output` through this callable.
*
* See `SummarizedCallable::propagatesFlow` for details.
*/
predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
none()
}
/**
* Gets the type of the parameter at the specified position with -1 indicating
* the instance parameter. If no types are provided then the types default to
* `Object`.
*/
Type getParameterType(int pos) { none() }
/**
* Gets the return type of this callable. If no type is provided then the type
* defaults to `Object`.
*/
Type getReturnType() { none() }
}
private newtype TSummarizedCallableBase =
TSimpleCallable(Callable c) { c.isSourceDeclaration() } or
TSyntheticCallable(SyntheticCallable c)
/**
* A callable that may have a flow summary. This is either a regular `Callable`
* or a `SyntheticCallable`.
*/
class SummarizedCallableBase extends TSummarizedCallableBase {
/** Gets a textual representation of this callable. */
string toString() { result = this.asCallable().toString() or result = this.asSyntheticCallable() }
/** Gets the source location for this callable. */
Location getLocation() {
result = this.asCallable().getLocation()
or
result.hasLocationInfo("", 0, 0, 0, 0) and
this instanceof TSyntheticCallable
}
/** Gets this callable cast as a `Callable`. */
Callable asCallable() { this = TSimpleCallable(result) }
/** Gets this callable cast as a `SyntheticCallable`. */
SyntheticCallable asSyntheticCallable() { this = TSyntheticCallable(result) }
/** Gets a call that targets this callable. */
Call getACall() {
result.getCallee().getSourceDeclaration() = this.asCallable()
or
result = this.asSyntheticCallable().getACall()
}
/**
* Gets the type of the parameter at the specified position with -1 indicating
* the instance parameter.
*/
Type getParameterType(int pos) {
result = this.asCallable().getParameterType(pos)
or
pos = -1 and result = this.asCallable().getDeclaringType()
or
result = this.asSyntheticCallable().getParameterType(pos)
or
exists(SyntheticCallable sc | sc = this.asSyntheticCallable() |
Impl::Private::summaryParameterNodeRange(this, pos) and
not exists(sc.getParameterType(pos)) and
result instanceof TypeObject
)
}
/** Gets the return type of this callable. */
Type getReturnType() {
result = this.asCallable().getReturnType()
or
exists(SyntheticCallable sc | sc = this.asSyntheticCallable() |
result = sc.getReturnType()
or
not exists(sc.getReturnType()) and
result instanceof TypeObject
)
}
}
class SummarizedCallable = Impl::Public::SummarizedCallable;
/**
* An adapter class to add the flow summaries specified on `SyntheticCallable`
* to `SummarizedCallable`.
*/
private class SummarizedSyntheticCallableAdapter extends SummarizedCallable, TSyntheticCallable {
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
this.asSyntheticCallable().propagatesFlow(input, output, preservesValue)
}
}
class RequiredSummaryComponentStack = Impl::Public::RequiredSummaryComponentStack;

8
java/ql/lib/semmle/code/java/dataflow/internal/DataFlowDispatch.qll
View File

@@ -9,9 +9,7 @@ private import semmle.code.java.dispatch.internal.Unification
private module DispatchImpl {
private predicate hasHighConfidenceTarget(Call c) {
exists(SummarizedCallable sc |
sc = c.getCallee().getSourceDeclaration() and not sc.isAutoGenerated()
)
exists(SummarizedCallable sc | sc.getACall() = c and not sc.isAutoGenerated())
or
exists(Callable srcTgt |
srcTgt = VirtualDispatch::viableCallable(c) and
@@ -30,7 +28,7 @@ private module DispatchImpl {
DataFlowCallable viableCallable(DataFlowCall c) {
result.asCallable() = sourceDispatch(c.asCall())
or
result.asSummarizedCallable() = c.asCall().getCallee().getSourceDeclaration()
result.asSummarizedCallable().getACall() = c.asCall()
}
/**
@@ -144,7 +142,7 @@ private module DispatchImpl {
not Unification::failsUnification(t, t2)
)
or
result.asSummarizedCallable() = def
result.asSummarizedCallable().getACall() = ma
)
}

6
java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll
View File

@@ -463,11 +463,7 @@ module Private {
c.asSummarizedCallable() = sc and pos = pos_
}
Type getTypeImpl() {
result = sc.getParameter(pos_).getType()
or
pos_ = -1 and result = sc.getDeclaringType()
}
Type getTypeImpl() { result = sc.getParameterType(pos_) }
}
}

6
java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll
View File

@@ -241,12 +241,6 @@ class DataFlowCallable extends TDataFlowCallable {
Field asFieldScope() { this = TFieldScope(result) }
RefType getDeclaringType() {
result = this.asCallable().getDeclaringType() or
result = this.asSummarizedCallable().getDeclaringType() or
result = this.asFieldScope().getDeclaringType()
}
string toString() {
result = this.asCallable().toString() or
result = "Synthetic: " + this.asSummarizedCallable().toString() or

17
java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImplSpecific.qll
View File

@@ -9,12 +9,9 @@ private import DataFlowUtil
private import FlowSummaryImpl::Private
private import FlowSummaryImpl::Public
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSummary as FlowSummary
private module FlowSummaries {
private import semmle.code.java.dataflow.FlowSummary as F
}
class SummarizedCallableBase = Callable;
class SummarizedCallableBase = FlowSummary::SummarizedCallableBase;
DataFlowCallable inject(SummarizedCallable c) { result.asSummarizedCallable() = c }
@@ -67,14 +64,16 @@ private boolean isGenerated(string provenance) {
* `input`, output specification `output`, kind `kind`, and a flag `generated`
* stating whether the summary is autogenerated.
*/
predicate summaryElement(Callable c, string input, string output, string kind, boolean generated) {
predicate summaryElement(
SummarizedCallableBase c, string input, string output, string kind, boolean generated
) {
exists(
string namespace, string type, boolean subtypes, string name, string signature, string ext,
string provenance
|
summaryModel(namespace, type, subtypes, name, signature, ext, input, output, kind, provenance) and
generated = isGenerated(provenance) and
c = interpretElement(namespace, type, subtypes, name, signature, ext)
c.asCallable() = interpretElement(namespace, type, subtypes, name, signature, ext)
)
}
@@ -82,11 +81,11 @@ predicate summaryElement(Callable c, string input, string output, string kind, b
* Holds if a negative flow summary exists for `c`, which means that there is no
* flow through `c`. The flag `generated` states whether the summary is autogenerated.
*/
predicate negativeSummaryElement(Callable c, boolean generated) {
predicate negativeSummaryElement(SummarizedCallableBase c, boolean generated) {
exists(string namespace, string type, string name, string signature, string provenance |
negativeSummaryModel(namespace, type, name, signature, provenance) and
generated = isGenerated(provenance) and
c = interpretElement(namespace, type, false, name, signature, "")
c.asCallable() = interpretElement(namespace, type, false, name, signature, "")
)
}

97
java/ql/lib/semmle/code/java/frameworks/Stream.qll
View File

@@ -1,6 +1,101 @@
/** Definitions related to `java.util.stream`. */
private import semmle.code.java.dataflow.ExternalFlow
private import semmle.code.java.dataflow.FlowSummary
private class CollectCall extends MethodAccess {
CollectCall() {
this.getMethod()
.getSourceDeclaration()
.hasQualifiedName("java.util.stream", "Stream", "collect")
}
}
private class Collector extends MethodAccess {
Collector() {
this.getMethod().getDeclaringType().hasQualifiedName("java.util.stream", "Collectors")
}
predicate hasName(string name) { this.getMethod().hasName(name) }
}
private class CollectToContainer extends SyntheticCallable {
CollectToContainer() { this = "java.util.stream.collect()+Collectors.[toList,...]" }
override Call getACall() {
result
.(CollectCall)
.getArgument(0)
.(Collector)
.hasName([
"maxBy", "minBy", "toCollection", "toList", "toSet", "toUnmodifiableList",
"toUnmodifiableSet"
])
}
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
input = SummaryComponentStack::elementOf(SummaryComponentStack::qualifier()) and
output = SummaryComponentStack::elementOf(SummaryComponentStack::return()) and
preservesValue = true
}
}
private class CollectToJoining extends SyntheticCallable {
CollectToJoining() { this = "java.util.stream.collect()+Collectors.joining" }
override Call getACall() { result.(CollectCall).getArgument(0).(Collector).hasName("joining") }
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
input = SummaryComponentStack::elementOf(SummaryComponentStack::qualifier()) and
output = SummaryComponentStack::return() and
preservesValue = false
}
override Type getReturnType() { result instanceof TypeString }
}
private class CollectToGroupingBy extends SyntheticCallable {
CollectToGroupingBy() {
this = "java.util.stream.collect()+Collectors.[groupingBy(Function),...]"
}
override Call getACall() {
exists(Method m |
m = result.(CollectCall).getArgument(0).(Collector).getMethod() and
m.hasName(["groupingBy", "groupingByConcurrent", "partitioningBy"]) and
m.getNumberOfParameters() = 1
)
}
override predicate propagatesFlow(
SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
) {
input = SummaryComponentStack::elementOf(SummaryComponentStack::qualifier()) and
output =
SummaryComponentStack::elementOf(SummaryComponentStack::mapValueOf(SummaryComponentStack::return())) and
preservesValue = true
}
}
private class RequiredComponentStackForCollect extends RequiredSummaryComponentStack {
override predicate required(SummaryComponent head, SummaryComponentStack tail) {
head = SummaryComponent::element() and
tail = SummaryComponentStack::qualifier()
or
head = SummaryComponent::element() and
tail = SummaryComponentStack::return()
or
head = SummaryComponent::element() and
tail = SummaryComponentStack::mapValueOf(SummaryComponentStack::return())
or
head = SummaryComponent::mapValue() and
tail = SummaryComponentStack::return()
}
}
private class StreamModel extends SummaryModelCsv {
override predicate row(string s) {
@@ -19,7 +114,7 @@ private class StreamModel extends SummaryModelCsv {
"java.util.stream;Stream;true;collect;(Supplier,BiConsumer,BiConsumer);;Argument[1].Parameter[0];Argument[2].Parameter[0..1];value;manual",
"java.util.stream;Stream;true;collect;(Supplier,BiConsumer,BiConsumer);;Argument[2].Parameter[0..1];Argument[1].Parameter[0];value;manual",
"java.util.stream;Stream;true;collect;(Supplier,BiConsumer,BiConsumer);;Argument[-1].Element;Argument[1].Parameter[1];value;manual",
// Missing: collect(Collector<T,A,R> collector)
// collect(Collector<T,A,R> collector) is handled separately on a case-by-case basis as it is too complex for MaD
"java.util.stream;Stream;true;concat;(Stream,Stream);;Argument[0..1].Element;ReturnValue.Element;value;manual",
"java.util.stream;Stream;true;distinct;();;Argument[-1].Element;ReturnValue.Element;value;manual",
"java.util.stream;Stream;true;dropWhile;(Predicate);;Argument[-1].Element;Argument[0].Parameter[0];value;manual",

2
java/ql/src/Telemetry/ExternalApi.qll
View File

@@ -69,7 +69,7 @@ class ExternalApi extends Callable {
/** Holds if this API has a supported summary. */
predicate hasSummary() {
this instanceof SummarizedCallable or
this = any(SummarizedCallable sc).asCallable() or
TaintTracking::localAdditionalTaintStep(this.getAnInput(), _)
}

2
java/ql/src/Telemetry/UnsupportedExternalAPIs.ql
View File

@@ -14,7 +14,7 @@ import ExternalApi
private predicate relevant(ExternalApi api) {
not api.isUninteresting() and
not api.isSupported() and
not api instanceof FlowSummaryImpl::Public::NegativeSummarizedCallable
not api = any(FlowSummaryImpl::Public::NegativeSummarizedCallable nsc).asCallable()
}
from string apiName, int usages

30
java/ql/test/library-tests/dataflow/stream-collect/A.java Normal file
View File

@@ -0,0 +1,30 @@
import java.util.*;
import java.util.stream.*;
public class A {
String source() { return "source"; }
void sink(Object o) { }
void m() {
String[] xs = new String[] { source() };
Stream<String> s = Arrays.stream(xs);
sink(s.collect(Collectors.maxBy(null)).get()); // $ hasValueFlow
sink(s.collect(Collectors.minBy(null)).get()); // $ hasValueFlow
sink(s.collect(Collectors.toCollection(null)).iterator().next()); // $ hasValueFlow
sink(s.collect(Collectors.toList()).get(0)); // $ hasValueFlow
sink(s.collect(Collectors.toSet()).iterator().next()); // $ hasValueFlow
sink(s.collect(Collectors.toUnmodifiableList()).get(0)); // $ hasValueFlow
sink(s.collect(Collectors.toUnmodifiableSet()).iterator().next()); // $ hasValueFlow
// we don't attempt to cover weird things like this:
sink(s.collect(true ? Collectors.toList() : null).get(0)); // $ MISSING: hasValueFlow
sink(s.collect(Collectors.joining())); // $ hasTaintFlow
sink(s.collect(Collectors.groupingBy(null)).get(null).get(0)); // $ hasValueFlow
sink(s.collect(Collectors.groupingByConcurrent(null)).get(null).get(0)); // $ hasValueFlow
sink(s.collect(Collectors.partitioningBy(null)).get(null).get(0)); // $ hasValueFlow
}
}

0
java/ql/test/library-tests/dataflow/stream-collect/test.expected Normal file
View File

1
java/ql/test/library-tests/dataflow/stream-collect/test.ql Normal file
View File

@@ -0,0 +1 @@
import TestUtilities.InlineFlowTest

4
python/ql/lib/semmle/python/Frameworks.qll
View File

@@ -12,6 +12,7 @@ private import semmle.python.frameworks.Asyncpg
private import semmle.python.frameworks.ClickhouseDriver
private import semmle.python.frameworks.Cryptodome
private import semmle.python.frameworks.Cryptography
private import semmle.python.frameworks.Cx_Oracle
private import semmle.python.frameworks.data.ModelsAsData
private import semmle.python.frameworks.Dill
private import semmle.python.frameworks.Django
@@ -33,12 +34,15 @@ private import semmle.python.frameworks.MarkupSafe
private import semmle.python.frameworks.Multidict
private import semmle.python.frameworks.Mysql
private import semmle.python.frameworks.MySQLdb
private import semmle.python.frameworks.Oracledb
private import semmle.python.frameworks.Peewee
private import semmle.python.frameworks.Phoenixdb
private import semmle.python.frameworks.Psycopg2
private import semmle.python.frameworks.Pycurl
private import semmle.python.frameworks.Pydantic
private import semmle.python.frameworks.Pymssql
private import semmle.python.frameworks.PyMySQL
private import semmle.python.frameworks.Pyodbc
private import semmle.python.frameworks.Requests
private import semmle.python.frameworks.RestFramework
private import semmle.python.frameworks.Rsa

31
python/ql/lib/semmle/python/frameworks/Cx_Oracle.qll Normal file
View File

@@ -0,0 +1,31 @@
/**
* Provides classes modeling security-relevant aspects of the `cx_Oracle` PyPI package.
*
* See
* - https://github.com/oracle/python-cx_Oracle
* - https://pypi.org/project/cx-Oracle/
*/
private import python
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
/**
* Provides models for the `cx_Oracle` PyPI package.
*
* See
* - https://github.com/oracle/python-cx_Oracle
* - https://pypi.org/project/cx-Oracle/
*/
private module Cx_Oracle {
/**
* A model for Cx_Oracle as a module that implements PEP 249, providing ways to execute SQL statements
* against a database.
*/
class Cx_Oracle extends PEP249::PEP249ModuleApiNode {
Cx_Oracle() { this = API::moduleImport("cx_Oracle") }
}
}

31
python/ql/lib/semmle/python/frameworks/Oracledb.qll Normal file
View File

@@ -0,0 +1,31 @@
/**
* Provides classes modeling security-relevant aspects of the `oracledb` PyPI package.
*
* See
* - https://python-oracledb.readthedocs.io/en/latest/index.html
* - https://pypi.org/project/oracledb/
*/
private import python
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
/**
* Provides models for the `oracledb` PyPI package.
*
* See
* - https://python-oracledb.readthedocs.io/en/latest/index.html
* - https://pypi.org/project/oracledb/
*/
private module Oracledb {
/**
* A model for oracledb as a module that implements PEP 249, providing ways to execute SQL statements
* against a database.
*/
class Oracledb extends PEP249::PEP249ModuleApiNode {
Oracledb() { this = API::moduleImport("oracledb") }
}
}

31
python/ql/lib/semmle/python/frameworks/Phoenixdb.qll Normal file
View File

@@ -0,0 +1,31 @@
/**
* Provides classes modeling security-relevant aspects of the `phoenixdb` PyPI package.
*
* See
* - https://github.com/apache/phoenix-queryserver/tree/master/python-phoenixdb
* - https://pypi.org/project/phoenixdb/
*/
private import python
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
/**
* Provides models for the `phoenixdb` PyPI package.
*
* See
* - https://github.com/apache/phoenix-queryserver/tree/master/python-phoenixdb
* - https://pypi.org/project/phoenixdb/
*/
private module Phoenixdb {
/**
* A model for Phoenixdb as a module that implements PEP 249, providing ways to execute SQL statements
* against a database.
*/
class Phoenixdb extends PEP249::PEP249ModuleApiNode {
Phoenixdb() { this = API::moduleImport("phoenixdb") }
}
}

31
python/ql/lib/semmle/python/frameworks/Pyodbc.qll Normal file
View File

@@ -0,0 +1,31 @@
/**
* Provides classes modeling security-relevant aspects of the `pyodbc` PyPI package.
*
* See
* - https://github.com/mkleehammer/pyodbc/wiki
* - https://pypi.org/project/pyodbc/
*/
private import python
private import semmle.python.dataflow.new.DataFlow
private import semmle.python.dataflow.new.RemoteFlowSources
private import semmle.python.Concepts
private import semmle.python.ApiGraphs
private import semmle.python.frameworks.PEP249
/**
* Provides models for the `pyodbc` PyPI package.
*
* See
* - https://github.com/mkleehammer/pyodbc/wiki
* - https://pypi.org/project/pyodbc/
*/
private module Pyodbc {
/**
* A model for Pyodbc as a module that implements PEP 249, providing ways to execute SQL statements
* against a database.
*/
class Pyodbc extends PEP249::PEP249ModuleApiNode {
Pyodbc() { this = API::moduleImport("pyodbc") }
}
}

4
python/ql/src/change-notes/2022-10-12-cx_oracle-phoenixdb-pyodbc-modeling.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Added model of `cx_Oracle`, `oracledb`, `phonenixdb` and `pyodbc` PyPI packages as a SQL interface following PEP249, resulting in additional sinks for `py/sql-injection`.

0
python/ql/test/library-tests/frameworks/cx_Oracle/ConceptsTest.expected Normal file
View File

2
python/ql/test/library-tests/frameworks/cx_Oracle/ConceptsTest.ql Normal file
View File

@@ -0,0 +1,2 @@
import python
import experimental.meta.ConceptsTest

6
python/ql/test/library-tests/frameworks/cx_Oracle/pep249.py Normal file
View File

@@ -0,0 +1,6 @@
import cx_Oracle
connection = cx_Oracle.connect(user="hr", password="pwd",
dsn="dbhost.example.com/orclpdb1")
cursor = connection.cursor()
cursor.execute("some sql") # $ getSql="some sql"

0
python/ql/test/library-tests/frameworks/oracledb/ConceptsTest.expected Normal file
View File

2
python/ql/test/library-tests/frameworks/oracledb/ConceptsTest.ql Normal file
View File

@@ -0,0 +1,2 @@
import python
import experimental.meta.ConceptsTest

5
python/ql/test/library-tests/frameworks/oracledb/pep249.py Normal file
View File

@@ -0,0 +1,5 @@
import oracledb
connection = oracledb.connect(user="username", password="password", dsn="connectstring")
cursor = connection.cursor()
cursor.execute("some sql") # $ getSql="some sql"

0
python/ql/test/library-tests/frameworks/phoenixdb/ConceptsTest.expected Normal file
View File

2
python/ql/test/library-tests/frameworks/phoenixdb/ConceptsTest.ql Normal file
View File

@@ -0,0 +1,2 @@
import python
import experimental.meta.ConceptsTest

8
python/ql/test/library-tests/frameworks/phoenixdb/pep249.py Normal file
View File

@@ -0,0 +1,8 @@
import phoenixdb
import phoenixdb.cursor
database_url = 'http://localhost:8765/'
conn = phoenixdb.connect(database_url, autocommit=True)
cursor = conn.cursor()
cursor.execute("some sql") # $ getSql="some sql"

0
python/ql/test/library-tests/frameworks/pyodbc/ConceptsTest.expected Normal file
View File

2
python/ql/test/library-tests/frameworks/pyodbc/ConceptsTest.ql Normal file
View File

@@ -0,0 +1,2 @@
import python
import experimental.meta.ConceptsTest

6
python/ql/test/library-tests/frameworks/pyodbc/pep249.py Normal file
View File

@@ -0,0 +1,6 @@
import pyodbc
cnxn = pyodbc.connect('DSN=test;PWD=password')
cursor = cnxn.cursor()
cursor.execute("some sql") # $ getSql="some sql"

5
ruby/ql/lib/change-notes/2022-10-13-actiondispatch-request.md Normal file
View File

@@ -0,0 +1,5 @@
---
category: minorAnalysis
---
* More sources of remote input arising from methods on `ActionDispatch::Request`
are now recognised.

4
ruby/ql/lib/change-notes/2022-10-13-faraday-run-request.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The response value returned by the `Faraday#run_request` method is now also considered a source of remote input.

54
ruby/ql/lib/codeql/ruby/Concepts.qll
View File

@@ -290,6 +290,26 @@ module Http {
}
}
/** A kind of request input. */
class RequestInputKind extends string {
RequestInputKind() { this = ["parameter", "header", "body", "url", "cookie"] }
}
/** Input from the parameters of a request. */
RequestInputKind parameterInputKind() { result = "parameter" }
/** Input from the headers of a request. */
RequestInputKind headerInputKind() { result = "header" }
/** Input from the body of a request. */
RequestInputKind bodyInputKind() { result = "body" }
/** Input from the URL of a request. */
RequestInputKind urlInputKind() { result = "url" }
/** Input from the cookies of a request. */
RequestInputKind cookieInputKind() { result = "cookie" }
/**
* An access to a user-controlled HTTP request input. For example, the URL or body of a request.
* Instances of this class automatically become `RemoteFlowSource`s.
@@ -304,6 +324,32 @@ module Http {
* This is typically the name of the method that gives rise to this input.
*/
string getSourceType() { result = super.getSourceType() }
/**
* Gets the kind of the accessed input,
* Can be one of "parameter", "header", "body", "url", "cookie".
*/
RequestInputKind getKind() { result = super.getKind() }
/**
* Holds if this part of the request may be controlled by a third party,
* that is, an agent other than the one who sent the request.
*
* This is true for the URL, query parameters, and request body.
* These can be controlled by a malicious third party in the following scenarios:
*
* - The user clicks a malicious link or is otherwise redirected to a malicious URL.
* - The user visits a web site that initiates a form submission or AJAX request on their behalf.
*
* In these cases, the request is technically sent from the user's browser, but
* the user is not in direct control of the URL or POST body.
*
* Headers are never considered third-party controllable by this predicate, although the
* third party does have some control over the the Referer and Origin headers.
*/
predicate isThirdPartyControllable() {
this.getKind() = [parameterInputKind(), urlInputKind(), bodyInputKind()]
}
}
/** Provides a class for modeling new HTTP request inputs. */
@@ -321,6 +367,12 @@ module Http {
* This is typically the name of the method that gives rise to this input.
*/
abstract string getSourceType();
/**
* Gets the kind of the accessed input,
* Can be one of "parameter", "header", "body", "url", "cookie".
*/
abstract RequestInputKind getKind();
}
}
@@ -387,6 +439,8 @@ module Http {
RoutedParameter() { this.getParameter() = handler.getARoutedParameter() }
override string getSourceType() { result = handler.getFramework() + " RoutedParameter" }
override RequestInputKind getKind() { result = parameterInputKind() }
}
/**

17
ruby/ql/lib/codeql/ruby/ast/internal/Module.qll
View File

@@ -138,9 +138,26 @@ private module Cached {
cached
string resolveConstantWrite(ConstantWriteAccess c) { result = resolveConstantWriteAccess(c) }
/**
* Gets a method named `name` that is available in module `m`. This includes methods
* that are included/prepended into `m` and methods available on base classes of `m`.
*/
cached
Method lookupMethod(Module m, string name) { TMethod(result) = lookupMethodOrConst(m, name) }
/**
* Gets a method named `name` that is available in a sub class of module `m`. This
* includes methods that are included/prepended into any of the sub classes of `m`,
* but not methods inherited from base classes.
*/
cached
Method lookupMethodInSubClasses(Module m, string name) {
exists(Module sub | sub.getSuperClass() = m |
TMethod(result) = lookupMethodOrConst0(sub, name) or
result = lookupMethodInSubClasses(sub, name)
)
}
cached
Expr lookupConst(Module m, string name) {
TExpr(result) = lookupMethodOrConst(m, name)

312
ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowDispatch.qll
View File

@@ -151,17 +151,24 @@ private class NormalCall extends DataFlowCall, TNormalCall {
override Location getLocation() { result = c.getLocation() }
}
/** A call for which we want to compute call targets. */
private class RelevantCall extends CfgNodes::ExprNodes::CallCfgNode {
pragma[nomagic]
RelevantCall() {
// Temporarily disable operation resolution (due to bad performance)
not this.getExpr() instanceof Operation
}
}
pragma[nomagic]
private predicate methodCall(
CfgNodes::ExprNodes::CallCfgNode call, DataFlow::Node receiver, string method
) {
private predicate methodCall(RelevantCall call, DataFlow::Node receiver, string method) {
method = call.getExpr().(MethodCall).getMethodName() and
receiver.asExpr() = call.getReceiver()
}
pragma[nomagic]
private predicate flowsToMethodCall(
CfgNodes::ExprNodes::CallCfgNode call, DataFlow::LocalSourceNode sourceNode, string method
private predicate flowsToMethodCallReceiver(
RelevantCall call, DataFlow::LocalSourceNode sourceNode, string method
) {
exists(DataFlow::Node receiver |
methodCall(call, receiver, method) and
@@ -169,7 +176,12 @@ private predicate flowsToMethodCall(
)
}
private Block yieldCall(CfgNodes::ExprNodes::CallCfgNode call) {
pragma[nomagic]
private predicate moduleFlowsToMethodCallReceiver(RelevantCall call, Module m, string method) {
flowsToMethodCallReceiver(call, trackModuleAccess(m), method)
}
private Block yieldCall(RelevantCall call) {
call.getExpr() instanceof YieldCall and
exists(BlockParameterNode node |
node = trackBlock(result) and
@@ -178,7 +190,7 @@ private Block yieldCall(CfgNodes::ExprNodes::CallCfgNode call) {
}
pragma[nomagic]
private predicate superCall(CfgNodes::ExprNodes::CallCfgNode call, Module superClass, string method) {
private predicate superCall(RelevantCall call, Module superClass, string method) {
call.getExpr() instanceof SuperCall and
exists(Module tp |
tp = call.getExpr().getEnclosingModule().getModule() and
@@ -187,20 +199,6 @@ private predicate superCall(CfgNodes::ExprNodes::CallCfgNode call, Module superC
)
}
pragma[nomagic]
private predicate instanceMethodCall(CfgNodes::ExprNodes::CallCfgNode call, Module tp, string method) {
exists(DataFlow::Node receiver, Module m, boolean exact |
methodCall(call, receiver, method) and
receiver = trackInstance(m, exact)
|
tp = m
or
// When we don't know the exact type, it could be any sub class
exact = false and
tp.getSuperClass+() = m
)
}
/** Holds if `self` belongs to module `m`. */
pragma[nomagic]
private predicate selfInModule(SelfVariable self, Module m) {
@@ -318,6 +316,19 @@ private predicate extendCallModule(Module m, Module n) {
)
}
/**
* Gets a method available in module `m`, or in one of `m`'s transitive
* sub classes when `exact = false`.
*/
pragma[nomagic]
private Method lookupMethod(Module m, string name, boolean exact) {
result = lookupMethod(m, name) and
exact in [false, true]
or
result = lookupMethodInSubClasses(m, name) and
exact = false
}
cached
private module Cached {
cached
@@ -332,110 +343,139 @@ private module Cached {
FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
}
cached
CfgScope getTarget(CfgNodes::ExprNodes::CallCfgNode call) {
// Temporarily disable operation resolution (due to bad performance)
not call.getExpr() instanceof Operation and
(
exists(string method |
exists(Module tp |
instanceMethodCall(call, tp, method) and
result = lookupMethod(tp, method) and
(
if result.(Method).isPrivate()
then
call.getReceiver().getExpr() instanceof SelfVariableAccess and
// For now, we restrict the scope of top-level declarations to their file.
// This may remove some plausible targets, but also removes a lot of
// implausible targets
if result.getEnclosingModule() instanceof Toplevel
then result.getFile() = call.getFile()
else any()
else any()
) and
if result.(Method).isProtected()
then result = lookupMethod(call.getExpr().getEnclosingModule().getModule(), method)
else any()
)
or
// singleton method defined on an instance, e.g.
// ```rb
// c = C.new
// def c.singleton; end # <- result
// c.singleton # <- call
// ```
// or an `extend`ed instance, e.g.
// ```rb
// c = C.new
// module M
// def instance; end # <- result
// end
// c.extend M
// c.instance # <- call
// ```
exists(DataFlow::Node receiver |
methodCall(call, receiver, method) and
receiver = trackSingletonMethodOnInstance(result, method)
)
or
// singleton method defined on a module
// or an `extend`ed module, e.g.
// ```rb
// module M
// def instance; end # <- result
// end
// M.extend(M)
// M.instance # <- call
// ```
exists(DataFlow::Node sourceNode, Module m |
flowsToMethodCall(call, sourceNode, method) and
result = lookupSingletonMethod(m, method)
|
// ```rb
// def C.singleton; end # <- result
// C.singleton # <- call
// ```
sourceNode = trackModuleAccess(m)
or
// ```rb
// class C
// def self.singleton; end # <- result
// self.singleton # <- call
// end
// ```
selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), m)
or
// ```rb
// class C
// def self.singleton; end # <- result
// def self.other
// self.singleton # <- call
// end
// end
// ```
exists(Module target, MethodBase caller |
target = m.getSuperClass*() and
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), caller, target) and
singletonMethod(caller, _, _) and
// Singleton methods declared in a block in the top-level may spuriously end up being seen as singleton
// methods on Object, if the block is actually evaluated in the context of another class.
// The 'self' inside such a singleton method could then be any class, leading to self-calls
// being resolved to arbitrary singleton methods.
// To remedy this, we do not allow following super-classes all the way to Object.
not (m != target and target = TResolved("Object"))
)
)
)
or
exists(Module superClass, string method |
superCall(call, superClass, method) and
result = lookupMethod(superClass, method)
)
or
result = yieldCall(call)
pragma[nomagic]
private Method lookupInstanceMethodCall(RelevantCall call, string method, boolean exact) {
exists(Module tp, DataFlow::Node receiver |
methodCall(call, pragma[only_bind_into](receiver), pragma[only_bind_into](method)) and
receiver = trackInstance(tp, exact) and
result = lookupMethod(tp, pragma[only_bind_into](method), exact)
)
}
pragma[nomagic]
private predicate isToplevelMethodInFile(Method m, File f) {
m.getEnclosingModule() instanceof Toplevel and
f = m.getFile()
}
/** Holds if a `self` access may be the receiver of `call` directly inside module `m`. */
pragma[nomagic]
private predicate selfInModuleFlowsToMethodCallReceiver(RelevantCall call, Module m, string method) {
exists(SsaSelfDefinitionNode self |
flowsToMethodCallReceiver(call, self, method) and
selfInModule(self.getVariable(), m)
)
}
/**
* Holds if a `self` access may be the receiver of `call` inside some singleton method, where
* that method belongs to `m` or one of `m`'s transitive super classes.
*/
pragma[nomagic]
private predicate selfInSingletonMethodFlowsToMethodCallReceiver(
RelevantCall call, Module m, string method
) {
exists(SsaSelfDefinitionNode self, Module target, MethodBase caller |
flowsToMethodCallReceiver(call, self, method) and
target = m.getSuperClass*() and
selfInMethod(self.getVariable(), caller, target) and
singletonMethod(caller, _, _) and
// Singleton methods declared in a block in the top-level may spuriously end up being seen as singleton
// methods on Object, if the block is actually evaluated in the context of another class.
// The 'self' inside such a singleton method could then be any class, leading to self-calls
// being resolved to arbitrary singleton methods.
// To remedy this, we do not allow following super-classes all the way to Object.
not (m != target and target = TResolved("Object"))
)
}
cached
CfgScope getTarget(RelevantCall call) {
exists(string method |
exists(boolean exact |
result = lookupInstanceMethodCall(call, method, exact) and
(
if result.(Method).isPrivate()
then
call.getReceiver().getExpr() instanceof SelfVariableAccess and
// For now, we restrict the scope of top-level declarations to their file.
// This may remove some plausible targets, but also removes a lot of
// implausible targets
(
isToplevelMethodInFile(result, call.getFile()) or
not isToplevelMethodInFile(result, _)
)
else any()
) and
if result.(Method).isProtected()
then result = lookupMethod(call.getExpr().getEnclosingModule().getModule(), method, exact)
else any()
)
or
// singleton method defined on an instance, e.g.
// ```rb
// c = C.new
// def c.singleton; end # <- result
// c.singleton # <- call
// ```
// or an `extend`ed instance, e.g.
// ```rb
// c = C.new
// module M
// def instance; end # <- result
// end
// c.extend M
// c.instance # <- call
// ```
exists(DataFlow::Node receiver |
methodCall(call, receiver, method) and
receiver = trackSingletonMethodOnInstance(result, method)
)
or
// singleton method defined on a module
// or an `extend`ed module, e.g.
// ```rb
// module M
// def instance; end # <- result
// end
// M.extend(M)
// M.instance # <- call
// ```
exists(Module m | result = lookupSingletonMethod(m, method) |
// ```rb
// def C.singleton; end # <- result
// C.singleton # <- call
// ```
moduleFlowsToMethodCallReceiver(call, m, method)
or
// ```rb
// class C
// def self.singleton; end # <- result
// self.singleton # <- call
// end
// ```
selfInModuleFlowsToMethodCallReceiver(call, m, method)
or
// ```rb
// class C
// def self.singleton; end # <- result
// def self.other
// self.singleton # <- call
// end
// end
// ```
selfInSingletonMethodFlowsToMethodCallReceiver(call, m, method)
)
)
or
exists(Module superClass, string method |
superCall(call, superClass, method) and
result = lookupMethod(superClass, method)
)
or
result = yieldCall(call)
}
/** Gets a viable run-time target for the call `call`. */
cached
DataFlowCallable viableCallable(DataFlowCall call) {
@@ -561,8 +601,8 @@ private predicate isInstance(DataFlow::Node n, Module tp, boolean exact) {
tp = TResolved("Proc") and
exact = true
or
exists(CfgNodes::ExprNodes::CallCfgNode call, DataFlow::LocalSourceNode sourceNode |
flowsToMethodCall(call, sourceNode, "new") and
exists(RelevantCall call, DataFlow::LocalSourceNode sourceNode |
flowsToMethodCallReceiver(call, sourceNode, "new") and
exact = true and
n.asExpr() = call
|
@@ -844,10 +884,7 @@ pragma[nomagic]
private predicate paramReturnFlow(
DataFlow::Node nodeFrom, DataFlow::PostUpdateNode nodeTo, StepSummary summary
) {
exists(
CfgNodes::ExprNodes::CallCfgNode call, DataFlow::Node arg, DataFlow::ParameterNode p,
Expr nodeFromPreExpr
|
exists(RelevantCall call, DataFlow::Node arg, DataFlow::ParameterNode p, Expr nodeFromPreExpr |
TypeTrackerSpecific::callStep(call, arg, p) and
nodeTo.getPreUpdateNode() = arg and
summary.toString() = "return" and
@@ -921,8 +958,7 @@ private predicate isInstanceLocalMustFlow(DataFlow::Node n, Module tp, boolean e
*/
pragma[nomagic]
private predicate mayBenefitFromCallContext0(
CfgNodes::ExprNodes::CallCfgNode ctx, ArgumentNode arg, CfgNodes::ExprNodes::CallCfgNode call,
Callable encl, string name
RelevantCall ctx, ArgumentNode arg, RelevantCall call, Callable encl, string name
) {
exists(
ParameterNodeImpl p, SsaDefinitionNode ssaNode, ParameterPosition ppos, ArgumentPosition apos
@@ -930,7 +966,7 @@ private predicate mayBenefitFromCallContext0(
// the receiver of `call` references `p`
ssaNode = trackInstance(_, _) and
LocalFlow::localFlowSsaParamInput(p, ssaNode) and
flowsToMethodCall(pragma[only_bind_into](call), pragma[only_bind_into](ssaNode),
flowsToMethodCallReceiver(pragma[only_bind_into](call), pragma[only_bind_into](ssaNode),
pragma[only_bind_into](name)) and
// `p` is a parameter of `encl`,
encl = call.getScope() and
@@ -953,8 +989,7 @@ private predicate mayBenefitFromCallContext0(
*/
pragma[nomagic]
private predicate mayBenefitFromCallContext1(
CfgNodes::ExprNodes::CallCfgNode ctx, CfgNodes::ExprNodes::CallCfgNode call, Callable encl,
Module tp, boolean exact, string name
RelevantCall ctx, RelevantCall call, Callable encl, Module tp, boolean exact, string name
) {
exists(ArgumentNode arg |
mayBenefitFromCallContext0(ctx, pragma[only_bind_into](arg), call, encl,
@@ -982,19 +1017,14 @@ predicate mayBenefitFromCallContext(DataFlowCall call, DataFlowCallable c) {
pragma[nomagic]
DataFlowCallable viableImplInCallContext(DataFlowCall call, DataFlowCall ctx) {
// `ctx` can provide a potentially better type bound
exists(CfgNodes::ExprNodes::CallCfgNode call0, Callable res |
exists(RelevantCall call0, Callable res |
call0 = call.asCall() and
res = result.asCallable() and
res = getTarget(call0) and // make sure to not include e.g. private methods
exists(Module tp, Module m, boolean exact, string name |
res = lookupMethod(tp, name) and
exists(Module m, boolean exact, string name |
res = lookupMethod(m, name, exact) and
mayBenefitFromCallContext1(ctx.asCall(), pragma[only_bind_into](call0), _,
pragma[only_bind_into](m), exact, pragma[only_bind_into](name))
|
tp = m
or
exact = false and
tp.getSuperClass+() = m
)
)
or

138
ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll
View File

@@ -139,6 +139,8 @@ class ParamsSource extends Http::Server::RequestInputAccess::Range {
ParamsSource() { this.asExpr().getExpr() instanceof Rails::ParamsCall }
override string getSourceType() { result = "ActionController::Metal#params" }
override Http::Server::RequestInputKind getKind() { result = Http::Server::parameterInputKind() }
}
/**
@@ -149,6 +151,8 @@ class CookiesSource extends Http::Server::RequestInputAccess::Range {
CookiesSource() { this.asExpr().getExpr() instanceof Rails::CookiesCall }
override string getSourceType() { result = "ActionController::Metal#cookies" }
override Http::Server::RequestInputKind getKind() { result = Http::Server::cookieInputKind() }
}
/** A call to `cookies` from within a controller. */
@@ -161,6 +165,140 @@ private class ActionControllerParamsCall extends ActionControllerContextCall, Pa
ActionControllerParamsCall() { this.getMethodName() = "params" }
}
/** Modeling for `ActionDispatch::Request`. */
private module Request {
/**
* A call to `request` from within a controller. This is an instance of
* `ActionDispatch::Request`.
*/
private class RequestNode extends DataFlow::CallNode {
RequestNode() {
this.asExpr().getExpr() instanceof ActionControllerContextCall and
this.getMethodName() = "request"
}
}
/**
* A method call on `request`.
*/
private class RequestMethodCall extends DataFlow::CallNode {
RequestMethodCall() {
any(RequestNode r).(DataFlow::LocalSourceNode).flowsTo(this.getReceiver())
}
}
abstract private class RequestInputAccess extends RequestMethodCall,
Http::Server::RequestInputAccess::Range {
override string getSourceType() { result = "ActionDispatch::Request#" + this.getMethodName() }
}
/**
* A method call on `request` which returns request parameters.
*/
private class ParametersCall extends RequestInputAccess {
ParametersCall() {
this.getMethodName() =
[
"parameters", "params", "GET", "POST", "query_parameters", "request_parameters",
"filtered_parameters"
]
}
override Http::Server::RequestInputKind getKind() {
result = Http::Server::parameterInputKind()
}
}
/** A method call on `request` which returns part or all of the request path. */
private class PathCall extends RequestInputAccess {
PathCall() {
this.getMethodName() =
["path", "filtered_path", "fullpath", "original_fullpath", "original_url", "url"]
}
override Http::Server::RequestInputKind getKind() { result = Http::Server::urlInputKind() }
}
/** A method call on `request` which returns a specific request header. */
private class HeadersCall extends RequestInputAccess {
HeadersCall() {
this.getMethodName() =
[
"authorization", "script_name", "path_info", "user_agent", "referer", "referrer",
"host_authority", "content_type", "host", "hostname", "accept_encoding",
"accept_language", "if_none_match", "if_none_match_etags", "content_mime_type"
]
or
// 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_.+")
}
override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
}
// TODO: each_header
/**
* A method call on `request` which returns part or all of the host.
* This can be influenced by headers such as Host and X-Forwarded-Host.
*/
private class HostCall extends RequestInputAccess {
HostCall() {
this.getMethodName() =
[
"authority", "host", "host_authority", "host_with_port", "hostname", "forwarded_for",
"forwarded_host", "port", "forwarded_port"
]
}
override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
}
/**
* A method call on `request` which is influenced by one or more request
* headers.
*/
private class HeaderTaintedCall extends RequestInputAccess {
HeaderTaintedCall() {
this.getMethodName() = ["media_type", "media_type_params", "content_charset", "base_url"]
}
override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
}
/** A method call on `request` which returns the request body. */
private class BodyCall extends RequestInputAccess {
BodyCall() { this.getMethodName() = ["body", "raw_post"] }
override Http::Server::RequestInputKind getKind() { result = Http::Server::bodyInputKind() }
}
/**
* A method call on `request` which returns the rack env.
* This is a hash containing all the information about the request. Values
* under keys starting with `HTTP_` are user-controlled.
*/
private class EnvCall extends RequestMethodCall {
EnvCall() { this.getMethodName() = ["env", "filtered_env"] }
}
/**
* A read of a user-controlled parameter from the request env.
*/
private class EnvHttpAccess extends DataFlow::CallNode, Http::Server::RequestInputAccess::Range {
EnvHttpAccess() {
any(EnvCall c).(DataFlow::LocalSourceNode).flowsTo(this.getReceiver()) and
this.getMethodName() = "[]" and
this.getArgument(0).asExpr().getExpr().getConstantValue().getString().regexpMatch("^HTTP_.+")
}
override Http::Server::RequestInputKind getKind() { result = Http::Server::headerInputKind() }
override string getSourceType() { result = "ActionDispatch::Request#env[]" }
}
}
/** A call to `render` from within a controller. */
private class ActionControllerRenderCall extends ActionControllerContextCall, RenderCallImpl {
ActionControllerRenderCall() { this.getMethodName() = "render" }

3
ruby/ql/lib/codeql/ruby/frameworks/http_clients/Faraday.qll
View File

@@ -37,7 +37,8 @@ class FaradayHttpRequest extends Http::Client::Request::Range, DataFlow::CallNod
API::getTopLevelMember("Faraday").getInstance()
] and
requestNode =
connectionNode.getReturn(["get", "head", "delete", "post", "put", "patch", "trace"]) and
connectionNode
.getReturn(["get", "head", "delete", "post", "put", "patch", "trace", "run_request"]) and
this = requestNode.asSource() and
connectionUse = connectionNode.asSource()
}

36
ruby/ql/lib/codeql/ruby/security/KernelOpenQuery.qll Normal file
View File

@@ -0,0 +1,36 @@
/**
* Provides utility classes and predicates for reasoning about `Kernel.open` and related methods.
*/
private import codeql.ruby.AST
private import codeql.ruby.DataFlow
private import codeql.ruby.AST
private import codeql.ruby.ApiGraphs
private import codeql.ruby.frameworks.core.Kernel::Kernel
/** A call to a method that might access a file or start a process. */
class AmbiguousPathCall extends DataFlow::CallNode {
string name;
AmbiguousPathCall() {
this.(KernelMethodCall).getMethodName() = "open" and
name = "Kernel.open"
or
this = API::getTopLevelMember("IO").getAMethodCall("read") and
not this = API::getTopLevelMember("File").getAMethodCall("read") and // needed in e.g. opal/opal, where some calls have both paths, but I'm not sure why
name = "IO.read"
}
/** Gets the name for the method being called. */
string getName() { result = name }
/** Gets the name for a safer method that can be used instead. */
string getReplacement() {
result = "File.read" and name = "IO.read"
or
result = "File.open" and name = "Kernel.open"
}
/** Gets the argument that specifies the path to be accessed. */
DataFlow::Node getPathArgument() { result = this.getArgument(0) }
}

4
ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll
View File

@@ -50,7 +50,9 @@ module UrlRedirect {
/**
* A source of remote user input, considered as a flow source.
*/
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
class HttpRequestInputAccessAsSource extends Source, Http::Server::RequestInputAccess {
HttpRequestInputAccessAsSource() { this.isThirdPartyControllable() }
}
/**
* A HTTP redirect response, considered as a flow sink.

6
ruby/ql/lib/codeql/ruby/security/XSS.qll
View File

@@ -312,9 +312,11 @@ module ReflectedXss {
deprecated predicate isAdditionalXSSTaintStep = isAdditionalXssTaintStep/2;
/**
* A source of remote user input, considered as a flow source.
* A HTTP request input, considered as a flow source.
*/
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
class HttpRequestInputAccessAsSource extends Source, Http::Server::RequestInputAccess {
HttpRequestInputAccessAsSource() { this.isThirdPartyControllable() }
}
}
/** DEPRECATED: Alias for ReflectedXss */

4
ruby/ql/src/change-notes/2022-10-06-non-constant-kernel-open.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `rb/non-constant-kernel-open`, to detect uses of Kernel.open and related methods with non-constant values.

46
ruby/ql/src/queries/security/cwe-078/KernelOpen.inc.qhelp Normal file
View File

@@ -0,0 +1,46 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>If <code>Kernel.open</code> is given a file name that starts with a <code>|</code>
character, it will execute the remaining string as a shell command. If a
malicious user can control the file name, they can execute arbitrary code.
The same vulnerability applies to <code>IO.read</code>.
</p>
</overview>
<recommendation>
<p>Use <code>File.open</code> instead of <code>Kernel.open</code>, as the former
does not have this vulnerability. Similarly, use <code>File.read</code> instead
of <code>IO.read</code>.</p>
</recommendation>
<example>
<p>
The following example shows code that calls <code>Kernel.open</code> on a
user-supplied file path.
</p>
<sample src="examples/kernel_open.rb" />
<p>Instead, <code>File.open</code> should be used, as in the following example.</p>
<sample src="examples/file_open.rb" />
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
</li>
<li>
Example CVE: <a href="https://www.ruby-lang.org/en/news/2021/05/02/os-command-injection-in-rdoc/">Command Injection in RDoc</a>.
</li>
</references>
</qhelp>

48
ruby/ql/src/queries/security/cwe-078/KernelOpen.qhelp
View File

@@ -1,46 +1,4 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>If <code>Kernel.open</code> is given a file name that starts with a <code>|</code>
character, it will execute the remaining string as a shell command. If a
malicious user can control the file name, they can execute arbitrary code.
The same vulnerability applies to <code>IO.read</code>.
</p>
</overview>
<recommendation>
<p>Use <code>File.open</code> instead of <code>Kernel.open</code>, as the former
does not have this vulnerability. Similarly, use <code>File.read</code> instead
of <code>IO.read</code>.</p>
</recommendation>
<example>
<p>
The following example shows code that calls <code>Kernel.open</code> on a
user-supplied file path.
</p>
<sample src="examples/kernel_open.rb" />
<p>Instead, <code>File.open</code> should be used, as in the following example.</p>
<sample src="examples/file_open.rb" />
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Command_Injection">Command Injection</a>.
</li>
<li>
Example CVE: <a href="https://www.ruby-lang.org/en/news/2021/05/02/os-command-injection-in-rdoc/">Command Injection in RDoc</a>.
</li>
</references>
</qhelp>
<include src="KernelOpen.inc.qhelp" />
</qhelp>

46
ruby/ql/src/queries/security/cwe-078/KernelOpen.ql
View File

@@ -1,5 +1,5 @@
/**
* @name Use of `Kernel.open` or `IO.read`
* @name Use of `Kernel.open` or `IO.read` with user-controlled input
* @description Using `Kernel.open` or `IO.read` may allow a malicious
* user to execute arbitrary system commands.
* @kind path-problem
@@ -14,39 +14,12 @@
* external/cwe/cwe-073
*/
import codeql.ruby.AST
import codeql.ruby.ApiGraphs
import codeql.ruby.frameworks.core.Kernel::Kernel
import codeql.ruby.TaintTracking
import codeql.ruby.dataflow.BarrierGuards
import codeql.ruby.dataflow.RemoteFlowSources
import codeql.ruby.DataFlow
import codeql.ruby.TaintTracking
import codeql.ruby.dataflow.RemoteFlowSources
import codeql.ruby.dataflow.BarrierGuards
import DataFlow::PathGraph
/**
* A method call that has a suggested replacement.
*/
abstract class Replacement extends DataFlow::CallNode {
abstract string getFrom();
abstract string getTo();
}
class KernelOpenCall extends KernelMethodCall, Replacement {
KernelOpenCall() { this.getMethodName() = "open" }
override string getFrom() { result = "Kernel.open" }
override string getTo() { result = "File.open" }
}
class IOReadCall extends DataFlow::CallNode, Replacement {
IOReadCall() { this = API::getTopLevelMember("IO").getAMethodCall("read") }
override string getFrom() { result = "IO.read" }
override string getTo() { result = "File.read" }
}
import codeql.ruby.security.KernelOpenQuery
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "KernelOpen" }
@@ -54,9 +27,7 @@ class Configuration extends TaintTracking::Configuration {
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
override predicate isSink(DataFlow::Node sink) {
exists(KernelOpenCall c | c.getArgument(0) = sink)
or
exists(IOReadCall c | c.getArgument(0) = sink)
sink = any(AmbiguousPathCall r).getPathArgument()
}
override predicate isSanitizer(DataFlow::Node node) {
@@ -73,5 +44,6 @@ where
sourceNode = source.getNode() and
call.getArgument(0) = sink.getNode()
select sink.getNode(), source, sink,
"This call to " + call.(Replacement).getFrom() + " depends on a $@. Replace it with " +
call.(Replacement).getTo() + ".", source.getNode(), "user-provided value"
"This call to " + call.(AmbiguousPathCall).getName() +
" depends on a $@. Consider replacing it with " + call.(AmbiguousPathCall).getReplacement() +
".", source.getNode(), "user-provided value"

4
ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.qhelp Normal file
View File

@@ -0,0 +1,4 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<include src="KernelOpen.inc.qhelp" />
</qhelp>

29
ruby/ql/src/queries/security/cwe-078/NonConstantKernelOpen.ql Normal file
View File

@@ -0,0 +1,29 @@
/**
* @name Use of `Kernel.open` or `IO.read` with a non-constant value
* @description Using `Kernel.open` or `IO.read` may allow a malicious
* user to execute arbitrary system commands.
* @kind problem
* @problem.severity warning
* @security-severity 6.5
* @precision high
* @id rb/non-constant-kernel-open
* @tags correctness
* security
* external/cwe/cwe-078
* external/cwe/cwe-088
* external/cwe/cwe-073
*/
import codeql.ruby.security.KernelOpenQuery
import codeql.ruby.ast.Literal
from AmbiguousPathCall call
where
// there is not a constant string argument
not exists(call.getPathArgument().asExpr().getExpr().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
select call,
"Call to " + call.getName() + " with a non-constant value. Consider replacing it with " +
call.getReplacement() + "."

139
ruby/ql/test/library-tests/frameworks/ActionController.expected
View File

@@ -1,17 +1,19 @@
actionControllerControllerClasses
| action_controller/input_access.rb:1:1:50:3 | UsersController |
| action_controller/params_flow.rb:1:1:151:3 | MyController |
| active_record/ActiveRecord.rb:23:1:39:3 | FooController |
| active_record/ActiveRecord.rb:41:1:64:3 | BarController |
| active_record/ActiveRecord.rb:66:1:98:3 | BazController |
| active_record/ActiveRecord.rb:100:1:108:3 | AnnotatedController |
| active_storage/active_storage.rb:39:1:45:3 | PostsController |
| app/controllers/comments_controller.rb:1:1:7:3 | CommentsController |
| app/controllers/comments_controller.rb:1:1:14:3 | CommentsController |
| app/controllers/foo/bars_controller.rb:3:1:46:3 | BarsController |
| app/controllers/photos_controller.rb:1:1:4:3 | PhotosController |
| app/controllers/posts_controller.rb:1:1:10:3 | PostsController |
| app/controllers/tags_controller.rb:1:1:2:3 | TagsController |
| app/controllers/users/notifications_controller.rb:2:3:5:5 | NotificationsController |
actionControllerActionMethods
| action_controller/input_access.rb:2:3:49:5 | index |
| action_controller/params_flow.rb:2:3:4:5 | m1 |
| action_controller/params_flow.rb:6:3:8:5 | m2 |
| action_controller/params_flow.rb:10:3:12:5 | m2 |
@@ -59,8 +61,8 @@ actionControllerActionMethods
| active_record/ActiveRecord.rb:101:3:103:5 | index |
| active_record/ActiveRecord.rb:105:3:107:5 | unsafe_action |
| active_storage/active_storage.rb:40:3:44:5 | create |
| app/controllers/comments_controller.rb:2:3:3:5 | index |
| app/controllers/comments_controller.rb:5:3:6:5 | show |
| app/controllers/comments_controller.rb:2:3:10:5 | index |
| app/controllers/comments_controller.rb:12:3:13:5 | show |
| app/controllers/foo/bars_controller.rb:5:3:7:5 | index |
| app/controllers/foo/bars_controller.rb:9:3:18:5 | show_debug |
| app/controllers/foo/bars_controller.rb:20:3:24:5 | show |
@@ -222,6 +224,137 @@ paramsSources
| app/controllers/foo/bars_controller.rb:21:21:21:26 | call to params |
| app/controllers/foo/bars_controller.rb:22:10:22:15 | call to params |
| app/views/foo/bars/show.html.erb:5:9:5:14 | call to params |
httpInputAccesses
| action_controller/input_access.rb:3:5:3:18 | call to params | ActionDispatch::Request#params |
| action_controller/input_access.rb:4:5:4:22 | call to parameters | ActionDispatch::Request#parameters |
| action_controller/input_access.rb:5:5:5:15 | call to GET | ActionDispatch::Request#GET |
| action_controller/input_access.rb:6:5:6:16 | call to POST | ActionDispatch::Request#POST |
| action_controller/input_access.rb:7:5:7:28 | call to query_parameters | ActionDispatch::Request#query_parameters |
| action_controller/input_access.rb:8:5:8:30 | call to request_parameters | ActionDispatch::Request#request_parameters |
| action_controller/input_access.rb:9:5:9:31 | call to filtered_parameters | ActionDispatch::Request#filtered_parameters |
| action_controller/input_access.rb:11:5:11:25 | call to authorization | ActionDispatch::Request#authorization |
| action_controller/input_access.rb:12:5:12:23 | call to script_name | ActionDispatch::Request#script_name |
| action_controller/input_access.rb:13:5:13:21 | call to path_info | ActionDispatch::Request#path_info |
| action_controller/input_access.rb:14:5:14:22 | call to user_agent | ActionDispatch::Request#user_agent |
| action_controller/input_access.rb:15:5:15:19 | call to referer | ActionDispatch::Request#referer |
| action_controller/input_access.rb:16:5:16:20 | call to referrer | ActionDispatch::Request#referrer |
| action_controller/input_access.rb:17:5:17:26 | call to host_authority | ActionDispatch::Request#host_authority |
| action_controller/input_access.rb:18:5:18:24 | call to content_type | ActionDispatch::Request#content_type |
| action_controller/input_access.rb:19:5:19:16 | call to host | ActionDispatch::Request#host |
| action_controller/input_access.rb:20:5:20:20 | call to hostname | ActionDispatch::Request#hostname |
| action_controller/input_access.rb:21:5:21:27 | call to accept_encoding | ActionDispatch::Request#accept_encoding |
| action_controller/input_access.rb:22:5:22:27 | call to accept_language | ActionDispatch::Request#accept_language |
| action_controller/input_access.rb:23:5:23:25 | call to if_none_match | ActionDispatch::Request#if_none_match |
| action_controller/input_access.rb:24:5:24:31 | call to if_none_match_etags | ActionDispatch::Request#if_none_match_etags |
| action_controller/input_access.rb:25:5:25:29 | call to content_mime_type | ActionDispatch::Request#content_mime_type |
| action_controller/input_access.rb:27:5:27:21 | call to authority | ActionDispatch::Request#authority |
| action_controller/input_access.rb:28:5:28:16 | call to host | ActionDispatch::Request#host |
| action_controller/input_access.rb:29:5:29:26 | call to host_authority | ActionDispatch::Request#host_authority |
| action_controller/input_access.rb:30:5:30:26 | call to host_with_port | ActionDispatch::Request#host_with_port |
| action_controller/input_access.rb:31:5:31:20 | call to hostname | ActionDispatch::Request#hostname |
| action_controller/input_access.rb:32:5:32:25 | call to forwarded_for | ActionDispatch::Request#forwarded_for |
| action_controller/input_access.rb:33:5:33:26 | call to forwarded_host | ActionDispatch::Request#forwarded_host |
| action_controller/input_access.rb:34:5:34:16 | call to port | ActionDispatch::Request#port |
| action_controller/input_access.rb:35:5:35:26 | call to forwarded_port | ActionDispatch::Request#forwarded_port |
| action_controller/input_access.rb:37:5:37:22 | call to media_type | ActionDispatch::Request#media_type |
| action_controller/input_access.rb:38:5:38:29 | call to media_type_params | ActionDispatch::Request#media_type_params |
| action_controller/input_access.rb:39:5:39:27 | call to content_charset | ActionDispatch::Request#content_charset |
| action_controller/input_access.rb:40:5:40:20 | call to base_url | ActionDispatch::Request#base_url |
| action_controller/input_access.rb:42:5:42:16 | call to body | ActionDispatch::Request#body |
| action_controller/input_access.rb:43:5:43:20 | call to raw_post | ActionDispatch::Request#raw_post |
| action_controller/input_access.rb:45:5:45:30 | ...[...] | ActionDispatch::Request#env[] |
| action_controller/input_access.rb:47:5:47:39 | ...[...] | ActionDispatch::Request#env[] |
| action_controller/params_flow.rb:3:10:3:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:7:10:7:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:11:10:11:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:15:10:15:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:19:10:19:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:23:10:23:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:27:10:27:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:31:10:31:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:35:10:35:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:39:10:39:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:43:10:43:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:47:10:47:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:51:10:51:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:55:10:55:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:59:10:59:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:63:10:63:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:67:10:67:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:71:10:71:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:75:10:75:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:79:10:79:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:83:10:83:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:87:10:87:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:91:10:91:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:95:10:95:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:99:10:99:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:103:10:103:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:107:10:107:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:111:10:111:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:112:23:112:28 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:116:10:116:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:117:31:117:36 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:121:10:121:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:122:31:122:36 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:126:10:126:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:127:24:127:29 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:130:14:130:19 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:135:10:135:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:136:32:136:37 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:139:22:139:27 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:144:10:144:15 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:145:32:145:37 | call to params | ActionController::Metal#params |
| action_controller/params_flow.rb:148:22:148:27 | call to params | ActionController::Metal#params |
| action_mailer/mailer.rb:3:10:3:15 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:28:30:28:35 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:29:29:29:34 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:30:31:30:36 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:32:21:32:26 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:34:34:34:39 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:35:23:35:28 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:35:38:35:43 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:43:10:43:15 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:50:11:50:16 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:54:12:54:17 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:59:12:59:17 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:62:15:62:20 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:68:21:68:26 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:72:18:72:23 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:76:24:76:29 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:76:49:76:54 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:80:25:80:30 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:80:50:80:55 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:88:21:88:26 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:92:27:92:32 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:92:52:92:57 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:96:28:96:33 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:96:53:96:58 | call to params | ActionController::Metal#params |
| active_record/ActiveRecord.rb:106:59:106:64 | call to params | ActionController::Metal#params |
| active_storage/active_storage.rb:41:21:41:26 | call to params | ActionController::Metal#params |
| active_storage/active_storage.rb:42:24:42:29 | call to params | ActionController::Metal#params |
| app/controllers/comments_controller.rb:3:5:3:18 | call to params | ActionDispatch::Request#params |
| app/controllers/comments_controller.rb:4:5:4:22 | call to parameters | ActionDispatch::Request#parameters |
| app/controllers/comments_controller.rb:5:5:5:15 | call to GET | ActionDispatch::Request#GET |
| app/controllers/comments_controller.rb:6:5:6:16 | call to POST | ActionDispatch::Request#POST |
| app/controllers/comments_controller.rb:7:5:7:28 | call to query_parameters | ActionDispatch::Request#query_parameters |
| app/controllers/comments_controller.rb:8:5:8:30 | call to request_parameters | ActionDispatch::Request#request_parameters |
| app/controllers/comments_controller.rb:9:5:9:31 | call to filtered_parameters | ActionDispatch::Request#filtered_parameters |
| app/controllers/foo/bars_controller.rb:10:27:10:33 | call to cookies | ActionController::Metal#cookies |
| app/controllers/foo/bars_controller.rb:13:21:13:26 | call to params | ActionController::Metal#params |
| app/controllers/foo/bars_controller.rb:14:10:14:15 | call to params | ActionController::Metal#params |
| app/controllers/foo/bars_controller.rb:21:21:21:26 | call to params | ActionController::Metal#params |
| app/controllers/foo/bars_controller.rb:22:10:22:15 | call to params | ActionController::Metal#params |
| app/graphql/mutations/dummy.rb:5:24:5:25 | id | GraphQL RoutedParameter |
| app/graphql/mutations/dummy.rb:9:17:9:25 | something | GraphQL RoutedParameter |
| app/graphql/resolvers/dummy_resolver.rb:6:24:6:25 | id | GraphQL RoutedParameter |
| app/graphql/resolvers/dummy_resolver.rb:10:17:10:25 | something | GraphQL RoutedParameter |
| app/graphql/types/query_type.rb:10:18:10:23 | number | GraphQL RoutedParameter |
| app/graphql/types/query_type.rb:18:23:18:33 | blah_number | GraphQL RoutedParameter |
| app/graphql/types/query_type.rb:27:20:27:25 | **args | GraphQL RoutedParameter |
| app/graphql/types/query_type.rb:36:34:36:37 | arg1 | GraphQL RoutedParameter |
| app/graphql/types/query_type.rb:36:41:36:46 | **rest | GraphQL RoutedParameter |
| app/views/foo/bars/show.html.erb:5:9:5:14 | call to params | ActionController::Metal#params |
cookiesCalls
| app/controllers/foo/bars_controller.rb:10:27:10:33 | call to cookies |
cookiesSources

6
ruby/ql/test/library-tests/frameworks/ActionController.ql
View File

@@ -1,6 +1,8 @@
private import codeql.ruby.AST
private import codeql.ruby.frameworks.ActionController
private import codeql.ruby.frameworks.Rails
private import codeql.ruby.frameworks.ActionView
private import codeql.ruby.Concepts
query predicate actionControllerControllerClasses(ActionControllerControllerClass cls) { any() }
@@ -10,6 +12,10 @@ query predicate paramsCalls(Rails::ParamsCall c) { any() }
query predicate paramsSources(ParamsSource src) { any() }
query predicate httpInputAccesses(Http::Server::RequestInputAccess a, string sourceType) {
sourceType = a.getSourceType()
}
query predicate cookiesCalls(Rails::CookiesCall c) { any() }
query predicate cookiesSources(CookiesSource src) { any() }

4
ruby/ql/test/library-tests/frameworks/ActionDispatch.expected
View File

@@ -36,8 +36,8 @@ actionDispatchRoutes
actionDispatchControllerMethods
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:2:3:8:5 | call to resources | app/controllers/posts_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:3:5 | index |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:5:3:6:5 | show |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:2:3:10:5 | index |
| app/config/routes.rb:3:5:6:7 | call to resources | app/controllers/comments_controller.rb:12:3:13:5 | show |
| app/config/routes.rb:7:5:7:37 | call to post | app/controllers/posts_controller.rb:8:3:9:5 | upvote |
| app/config/routes.rb:27:3:27:48 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |
| app/config/routes.rb:28:3:28:50 | call to match | app/controllers/photos_controller.rb:2:3:3:5 | show |

50
ruby/ql/test/library-tests/frameworks/action_controller/input_access.rb Normal file
View File

@@ -0,0 +1,50 @@
class UsersController < ActionController::Base
def index
request.params
request.parameters
request.GET
request.POST
request.query_parameters
request.request_parameters
request.filtered_parameters
request.authorization
request.script_name
request.path_info
request.user_agent
request.referer
request.referrer
request.host_authority
request.content_type
request.host
request.hostname
request.accept_encoding
request.accept_language
request.if_none_match
request.if_none_match_etags
request.content_mime_type
request.authority
request.host
request.host_authority
request.host_with_port
request.hostname
request.forwarded_for
request.forwarded_host
request.port
request.forwarded_port
request.media_type
request.media_type_params
request.content_charset
request.base_url
request.body
request.raw_post
request.env["HTTP_ACCEPT"]
request.env["NOT_USER_CONTROLLED"]
request.filtered_env["HTTP_ACCEPT"]
request.filtered_env["NOT_USER_CONTROLLED"]
end
end

9
ruby/ql/test/library-tests/frameworks/app/controllers/comments_controller.rb
View File

@@ -1,7 +1,14 @@
class CommentsController < ApplicationController
def index
request.params
request.parameters
request.GET
request.POST
request.query_parameters
request.request_parameters
request.filtered_parameters
end
def show
end
end
end

0
ruby/ql/test/query-tests/security/cwe-078/CommandInjection.expected → ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected
View File

0
ruby/ql/test/query-tests/security/cwe-078/CommandInjection.qlref → ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.qlref
View File

0
ruby/ql/test/query-tests/security/cwe-078/CommandInjection.rb → ruby/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.rb
View File

4
ruby/ql/test/query-tests/security/cwe-078/KernelOpen.expected → ruby/ql/test/query-tests/security/cwe-078/KernelOpen/KernelOpen.expected
View File

@@ -9,5 +9,5 @@ nodes
| KernelOpen.rb:5:13:5:16 | file | semmle.label | file |
subpaths
#select
| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a $@. Replace it with File.open. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value |
| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a $@. Replace it with File.read. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value |
| KernelOpen.rb:4:10:4:13 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:4:10:4:13 | file | This call to Kernel.open depends on a $@. Consider replacing it with File.open. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value |
| KernelOpen.rb:5:13:5:16 | file | KernelOpen.rb:3:12:3:17 | call to params : | KernelOpen.rb:5:13:5:16 | file | This call to IO.read depends on a $@. Consider replacing it with File.read. | KernelOpen.rb:3:12:3:17 | call to params | user-provided value |

0
ruby/ql/test/query-tests/security/cwe-078/KernelOpen.qlref → ruby/ql/test/query-tests/security/cwe-078/KernelOpen/KernelOpen.qlref
View File

0
ruby/ql/test/query-tests/security/cwe-078/KernelOpen.rb → ruby/ql/test/query-tests/security/cwe-078/KernelOpen/KernelOpen.rb
View File

4
ruby/ql/test/query-tests/security/cwe-078/NonConstantKernelOpen/NonConstantKernelOpen.expected Normal file
View File

@@ -0,0 +1,4 @@
| NonConstantKernelOpen.rb:4:5:4:14 | call to open | Call to Kernel.open with a non-constant value. Consider replacing it with File.open. |
| NonConstantKernelOpen.rb:5:5:5:17 | call to read | Call to IO.read with a non-constant value. Consider replacing it with File.read. |
| NonConstantKernelOpen.rb:9:5:9:21 | call to open | Call to Kernel.open with a non-constant value. Consider replacing it with File.open. |
| NonConstantKernelOpen.rb:19:5:19:33 | call to open | Call to Kernel.open with a non-constant value. Consider replacing it with File.open. |

1
ruby/ql/test/query-tests/security/cwe-078/NonConstantKernelOpen/NonConstantKernelOpen.qlref Normal file
View File

@@ -0,0 +1 @@
queries/security/cwe-078/NonConstantKernelOpen.ql

23
ruby/ql/test/query-tests/security/cwe-078/NonConstantKernelOpen/NonConstantKernelOpen.rb Normal file
View File

@@ -0,0 +1,23 @@
class UsersController < ActionController::Base
def create
file = params[:file]
open(file) # BAD
IO.read(file) # BAD
File.open(file).read # GOOD
Kernel.open(file) # BAD
File.open(file, "r") # GOOD
Kernel.open("constant") # GOOD
IO.read("constant") # GOOD
Kernel.open("this is #{fine}") # GOOD
Kernel.open("#{this_is} bad") # BAD
open("| #{this_is_an_explicit_command} foo bar") # GOOD
end
end

14
ruby/ql/test/query-tests/security/cwe-079/ReflectedXSS.expected
View File

@@ -10,13 +10,13 @@ edges
| app/controllers/foo/bars_controller.rb:17:21:17:36 | ...[...] : | app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website |
| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : |
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : |
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : |
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : |
| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : | app/views/foo/bars/show.html.erb:41:3:41:16 | @instance_text |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text : |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:5:9:5:20 | call to display_text |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:8:9:8:36 | ...[...] |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:12:9:12:26 | ...[...] |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:36:3:36:14 | call to display_text |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text : |
| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text |
| app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... : | app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] |
| app/views/foo/bars/show.html.erb:44:76:44:87 | call to display_text : | app/views/foo/bars/show.html.erb:44:64:44:87 | ... + ... : |
@@ -35,7 +35,7 @@ nodes
| app/controllers/foo/bars_controller.rb:18:10:18:15 | call to params : | semmle.label | call to params : |
| app/controllers/foo/bars_controller.rb:18:10:18:22 | ...[...] : | semmle.label | ...[...] : |
| app/controllers/foo/bars_controller.rb:19:22:19:23 | dt : | semmle.label | dt : |
| app/controllers/foo/bars_controller.rb:23:53:23:54 | dt : | semmle.label | dt : |
| app/controllers/foo/bars_controller.rb:24:53:24:54 | dt : | semmle.label | dt : |
| app/views/foo/bars/_widget.html.erb:5:9:5:20 | call to display_text | semmle.label | call to display_text |
| app/views/foo/bars/_widget.html.erb:8:9:8:36 | ...[...] | semmle.label | ...[...] |
| app/views/foo/bars/show.html.erb:2:18:2:30 | @user_website | semmle.label | @user_website |

1
ruby/ql/test/query-tests/security/cwe-079/app/controllers/foo/bars_controller.rb
View File

@@ -20,6 +20,7 @@ class BarsController < ApplicationController
@safe_foo = params[:text]
@safe_foo = "safe_foo"
@html_escaped = ERB::Util.html_escape(params[:text])
@header_escaped = ERB::Util.html_escape(cookies[:foo]) # OK - cookies not controllable by 3rd party
render "foo/bars/show", locals: { display_text: dt, safe_text: "hello" }
end
end

10
ruby/ql/test/query-tests/security/cwe-601/UrlRedirect.expected
View File

@@ -3,14 +3,14 @@ edges
| UrlRedirect.rb:14:17:14:22 | call to params : | UrlRedirect.rb:14:17:14:43 | call to fetch |
| UrlRedirect.rb:19:17:19:22 | call to params : | UrlRedirect.rb:19:17:19:37 | call to to_unsafe_hash |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:88:21:88:32 | input_params : |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:93:21:93:32 | input_params : |
| UrlRedirect.rb:34:20:34:25 | call to params : | UrlRedirect.rb:34:20:34:31 | ...[...] : |
| UrlRedirect.rb:34:20:34:31 | ...[...] : | UrlRedirect.rb:34:17:34:37 | "#{...}/foo" |
| UrlRedirect.rb:58:17:58:22 | call to params : | UrlRedirect.rb:58:17:58:28 | ...[...] |
| UrlRedirect.rb:63:38:63:43 | call to params : | UrlRedirect.rb:63:38:63:49 | ...[...] |
| UrlRedirect.rb:68:38:68:43 | call to params : | UrlRedirect.rb:68:38:68:49 | ...[...] |
| UrlRedirect.rb:73:25:73:30 | call to params : | UrlRedirect.rb:73:25:73:36 | ...[...] |
| UrlRedirect.rb:88:21:88:32 | input_params : | UrlRedirect.rb:89:5:89:29 | call to permit : |
| UrlRedirect.rb:93:21:93:32 | input_params : | UrlRedirect.rb:94:5:94:29 | call to permit : |
nodes
| UrlRedirect.rb:4:17:4:22 | call to params | semmle.label | call to params |
| UrlRedirect.rb:9:17:9:22 | call to params : | semmle.label | call to params : |
@@ -32,10 +32,10 @@ nodes
| UrlRedirect.rb:68:38:68:49 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:73:25:73:30 | call to params : | semmle.label | call to params : |
| UrlRedirect.rb:73:25:73:36 | ...[...] | semmle.label | ...[...] |
| UrlRedirect.rb:88:21:88:32 | input_params : | semmle.label | input_params : |
| UrlRedirect.rb:89:5:89:29 | call to permit : | semmle.label | call to permit : |
| UrlRedirect.rb:93:21:93:32 | input_params : | semmle.label | input_params : |
| UrlRedirect.rb:94:5:94:29 | call to permit : | semmle.label | call to permit : |
subpaths
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:88:21:88:32 | input_params : | UrlRedirect.rb:89:5:89:29 | call to permit : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
| UrlRedirect.rb:24:31:24:36 | call to params : | UrlRedirect.rb:93:21:93:32 | input_params : | UrlRedirect.rb:94:5:94:29 | call to permit : | UrlRedirect.rb:24:17:24:37 | call to filter_params |
#select
| UrlRedirect.rb:4:17:4:22 | call to params | UrlRedirect.rb:4:17:4:22 | call to params | UrlRedirect.rb:4:17:4:22 | call to params | Untrusted URL redirection depends on a $@. | UrlRedirect.rb:4:17:4:22 | call to params | user-provided value |
| UrlRedirect.rb:9:17:9:28 | ...[...] | UrlRedirect.rb:9:17:9:22 | call to params : | UrlRedirect.rb:9:17:9:28 | ...[...] | Untrusted URL redirection depends on a $@. | UrlRedirect.rb:9:17:9:22 | call to params | user-provided value |

5
ruby/ql/test/query-tests/security/cwe-601/UrlRedirect.rb
View File

@@ -83,6 +83,11 @@ class UsersController < ActionController::Base
redirect_back_or_to params[:key], allow_other_host: false
end
# GOOD
def route15
redirect_to cookies[:foo]
end
private
def filter_params(input_params)

10
swift/codegen/generators/qlgen.py
View File

@@ -230,18 +230,18 @@ def _partition(l, pred):
return map(list, _partition_iter(l, pred))
def _is_in_qltest_collapsed_hierachy(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
return "qltest_collapse_hierarchy" in cls.pragmas or _is_under_qltest_collapsed_hierachy(cls, lookup)
def _is_in_qltest_collapsed_hierarchy(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
return "qltest_collapse_hierarchy" in cls.pragmas or _is_under_qltest_collapsed_hierarchy(cls, lookup)
def _is_under_qltest_collapsed_hierachy(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
def _is_under_qltest_collapsed_hierarchy(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
return "qltest_uncollapse_hierarchy" not in cls.pragmas and any(
_is_in_qltest_collapsed_hierachy(lookup[b], lookup) for b in cls.bases)
_is_in_qltest_collapsed_hierarchy(lookup[b], lookup) for b in cls.bases)
def _should_skip_qltest(cls: schema.Class, lookup: typing.Dict[str, schema.Class]):
return "qltest_skip" in cls.pragmas or not (
cls.final or "qltest_collapse_hierarchy" in cls.pragmas) or _is_under_qltest_collapsed_hierachy(
cls.final or "qltest_collapse_hierarchy" in cls.pragmas) or _is_under_qltest_collapsed_hierarchy(
cls, lookup)

2
swift/extractor/infra/SwiftDispatcher.h
View File

@@ -308,7 +308,7 @@ class SwiftDispatcher {
static FilePath getFilePath(llvm::StringRef path) {
// TODO: this needs more testing
// TODO: check canonicaliztion of names on a case insensitive filesystems
// TODO: check canonicalization of names on a case insensitive filesystems
// TODO: make symlink resolution conditional on CODEQL_PRESERVE_SYMLINKS=true
llvm::SmallString<PATH_MAX> realPath;
if (std::error_code ec = llvm::sys::fs::real_path(path, realPath)) {

2
swift/ql/lib/codeql/swift/controlflow/internal/Completion.qll
View File

@@ -94,7 +94,7 @@ private predicate isBooleanConstant(ControlFlowElement n, boolean value) {
mustHaveBooleanCompletion(n) and
value = n.asAstNode().(BooleanLiteralExpr).getValue()
or
// Boolean consants hidden inside conversions are also
// Boolean constants hidden inside conversions are also
// constants that resolve to the same value.
exists(ControlFlowElement parent |
parent.asAstNode() = n.asAstNode().getResolveStep() and

2
swift/ql/lib/codeql/swift/controlflow/internal/ControlFlowGraphImpl.qll
View File

@@ -1074,7 +1074,7 @@ module Exprs {
/**
* The control-flow for assignments where the left-hand side has
* direct-to-implmentation-access semantics.
* direct-to-implementation-access semantics.
*/
class PropertyAssignExpr extends AssignExprTree {
AccessorDecl accessorDecl;

4
swift/ql/lib/codeql/swift/dataflow/internal/DataFlowDispatch.qll
View File

@@ -71,7 +71,7 @@ newtype TDataFlowCall =
TNormalCall(ApplyExprCfgNode call) or
TPropertyGetterCall(PropertyGetterCfgNode getter) or
TPropertySetterCall(PropertySetterCfgNode setter) or
TPropertyObserverCall(PropertyObserverCfgNode obserer) or
TPropertyObserverCall(PropertyObserverCfgNode observer) or
TSummaryCall(FlowSummaryImpl::Public::SummarizedCallable c, Node receiver) {
FlowSummaryImpl::Private::summaryCallbackRange(c, receiver)
}
@@ -191,7 +191,7 @@ class PropertyObserverCall extends DataFlowCall, TPropertyObserverCall {
result = observer.getBase()
or
// TODO: This is correct for `willSet` (which takes a `newValue` parameter),
// but for `didSet` (which takes an `oldValue` paramter) we need an rvalue
// but for `didSet` (which takes an `oldValue` parameter) we need an rvalue
// for `getBase()`.
i = 0 and
result = observer.getSource()

2
swift/tools/tracing-config.lua
View File

@@ -12,7 +12,7 @@ function RegisterExtractorPack(id)
return nil
end
-- removes upsupported CLI arg including the following how_many args
-- removes unsupported CLI arg including the following how_many args
function strip_unsupported_arg(args, arg, how_many)
local index = indexOf(args, arg)
if index then