Merge branch 'master' into copymove

This commit is contained in:
Geoffrey White
2020-08-04 15:41:27 +01:00
225 changed files with 11138 additions and 3458 deletions

View File

@@ -1,5 +1,6 @@
{ "provide": [ "*/ql/src/qlpack.yml",
"*/ql/test/qlpack.yml",
"*/ql/examples/qlpack.yml",
"*/upgrades/qlpack.yml",
"misc/legacy-support/*/qlpack.yml",
"misc/suite-helpers/qlpack.yml" ] }

3
.vscode/settings.json vendored Normal file
View File

@@ -0,0 +1,3 @@
{
"omnisharp.autoStart": false
}

View File

@@ -28,27 +28,51 @@ The following changes in version 1.25 affect C# analysis in all applications.
such as `A<int>.B`, no longer are considered unbound generics. (Such nested types do,
however, still have relevant `.getSourceDeclaration()`s, for example `A<>.B`.)
* The data-flow library has been improved, which affects most security queries by potentially
adding more results. Flow through methods now takes nested field reads/writes into account.
For example, the library is able to track flow from `"taint"` to `Sink()` via the method
`GetF2F1()` in
```csharp
class C1
{
string F1;
}
adding more results:
- Flow through methods now takes nested field reads/writes into account.
For example, the library is able to track flow from `"taint"` to `Sink()` via the method
`GetF2F1()` in
```csharp
class C1
{
string F1;
}
class C2
{
C1 F2;
string GetF2F1() => F2.F1; // Nested field read
class C2
{
C1 F2;
void M()
{
F2 = new C1() { F1 = "taint" };
Sink(GetF2F1()); // NEW: "taint" reaches here
}
}
```
string GetF2F1() => F2.F1; // Nested field read
void M()
{
F2 = new C1() { F1 = "taint" };
Sink(GetF2F1()); // NEW: "taint" reaches here
}
}
```
- Flow through collections is now modeled precisely. For example, instead of modeling an array
store `a[i] = x` as a taint-step from `x` to `a`, we now model it as a data-flow step that
stores `x` into `a`. To get the value back out, a matching read step must be taken.
For source-code based data-flow analysis, the following constructs are modeled as stores into
collections:
- Direct array assignments, `a[i] = x`.
- Array initializers, `new [] { x }`.
- C# 6-style array initializers, `new C() { Array = { [i] = x } }`.
- Call arguments that match a `params` parameter, where the C# compiler creates an array under-the-hood.
- `yield return` statements.
The following source-code constructs read from a collection:
- Direct array reads, `a[i]`.
- `foreach` statements.
For calls out to library code, existing flow summaries have been refined to precisely
capture how they interact with collection contents. For example, a call to
`System.Collections.Generic.List<T>.Add(T)` stores the value of the argument into the
qualifier, and a call to `System.Collections.Generic.List<T>.get_Item(int)` (that is, an
indexer call) reads contents out of the qualifier. Moreover, the effect of
collection-clearing methods such as `System.Collections.Generic.List<T>.Clear()` is now
also modeled.
## Changes to autobuilder

View File

@@ -6,8 +6,10 @@
- [Promise](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise)
- [bluebird](http://bluebirdjs.com/)
- [express](https://www.npmjs.com/package/express)
- [execa](https://www.npmjs.com/package/execa)
- [fancy-log](https://www.npmjs.com/package/fancy-log)
- [fastify](https://www.npmjs.com/package/fastify)
- [foreground-child](https://www.npmjs.com/package/foreground-child)
- [fstream](https://www.npmjs.com/package/fstream)
- [jGrowl](https://github.com/stanlemon/jGrowl)
- [jQuery](https://jquery.com/)
@@ -17,6 +19,7 @@
- [mssql](https://www.npmjs.com/package/mssql)
- [mysql](https://www.npmjs.com/package/mysql)
- [npmlog](https://www.npmjs.com/package/npmlog)
- [opener](https://www.npmjs.com/package/opener)
- [pg](https://www.npmjs.com/package/pg)
- [sequelize](https://www.npmjs.com/package/sequelize)
- [spanner](https://www.npmjs.com/package/spanner)

View File

@@ -0,0 +1,3 @@
name: codeql-cpp-examples
version: 0.0.0
libraryPathDependencies: codeql-cpp

View File

@@ -9,3 +9,10 @@
tags contain:
- ide-contextual-queries/local-definitions
- ide-contextual-queries/local-references
- query: Metrics/Dependencies/ExternalDependencies.ql
- query: Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
- query: Metrics/Files/FLinesOfCode.ql
- query: Metrics/Files/FLinesOfCommentedOutCode.ql
- query: Metrics/Files/FLinesOfComments.ql
- query: Metrics/Files/FLinesOfDuplicatedCode.ql
- query: Metrics/Files/FNumberOfTests.ql

View File

@@ -197,7 +197,8 @@ class Element extends ElementBase {
initialisers(underlyingElement(this), unresolveElement(result), _, _) or
exprconv(unresolveElement(result), underlyingElement(this)) or
param_decl_bind(underlyingElement(this), _, unresolveElement(result)) or
using_container(unresolveElement(result), underlyingElement(this))
using_container(unresolveElement(result), underlyingElement(this)) or
static_asserts(unresolveElement(this), _, _, _, underlyingElement(result))
}
/** Gets the closest `Element` enclosing this one. */
@@ -278,12 +279,12 @@ class StaticAssert extends Locatable, @static_assert {
/**
* Gets the expression which this static assertion ensures is true.
*/
Expr getCondition() { static_asserts(underlyingElement(this), unresolveElement(result), _, _) }
Expr getCondition() { static_asserts(underlyingElement(this), unresolveElement(result), _, _, _) }
/**
* Gets the message which will be reported by the compiler if this static assertion fails.
*/
string getMessage() { static_asserts(underlyingElement(this), _, result, _) }
string getMessage() { static_asserts(underlyingElement(this), _, result, _, _) }
override Location getLocation() { static_asserts(underlyingElement(this), _, _, result) }
override Location getLocation() { static_asserts(underlyingElement(this), _, _, result, _) }
}

View File

@@ -582,7 +582,7 @@ class TemplateVariable extends Variable {
* float a;
* }
*
* template<type T>
* template<typename T>
* void myTemplateFunction() {
* T b;
* }

View File

@@ -49,6 +49,18 @@ predicate primitiveVariadicFormatter(TopLevelFunction f, int formatParamIndex) {
)
}
/**
* A standard function such as `vsprintf` that has an output parameter
* and a variable argument list of type `va_arg`.
*/
private predicate primitiveVariadicFormatterOutput(TopLevelFunction f, int outputParamIndex) {
// note: this might look like the regular expression in `primitiveVariadicFormatter`, but
// there is one important difference: the [fs] part is not optional, as these classify
// the `printf` variants that write to a buffer.
// Conveniently, these buffer parameters are all at index 0.
f.getName().regexpMatch("_?_?va?[fs]n?w?printf(_s)?(_p)?(_l)?") and outputParamIndex = 0
}
private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
exists(FunctionCall fc, int i |
variadicFormatter(fc.getTarget(), i) and
@@ -57,6 +69,26 @@ private predicate callsVariadicFormatter(Function f, int formatParamIndex) {
)
}
private predicate callsVariadicFormatterOutput(Function f, int outputParamIndex) {
exists(FunctionCall fc, int i |
fc.getEnclosingFunction() = f and
variadicFormatterOutput(fc.getTarget(), i) and
fc.getArgument(i) = f.getParameter(outputParamIndex).getAnAccess()
)
}
/**
* Holds if `f` is a function such as `vprintf` that takes variable argument list
* of type `va_arg` and writes formatted output to a buffer given as a parameter at
* index `outputParamIndex`, if any.
*/
private predicate variadicFormatterOutput(Function f, int outputParamIndex) {
primitiveVariadicFormatterOutput(f, outputParamIndex)
or
not f.isVarargs() and
callsVariadicFormatterOutput(f, outputParamIndex)
}
/**
* Holds if `f` is a function such as `vprintf` that has a format parameter
* (at `formatParamIndex`) and a variable argument list of type `va_arg`.
@@ -78,6 +110,8 @@ class UserDefinedFormattingFunction extends FormattingFunction {
UserDefinedFormattingFunction() { isVarargs() and callsVariadicFormatter(this, _) }
override int getFormatParameterIndex() { callsVariadicFormatter(this, result) }
override int getOutputParameterIndex() { callsVariadicFormatterOutput(this, result) }
}
/**

View File

@@ -699,13 +699,20 @@ predicate localExprFlow(Expr e1, Expr e2) { localFlow(exprNode(e1), exprNode(e2)
*/
class BarrierGuard extends IRGuardCondition {
/** Override this predicate to hold if this guard validates `instr` upon evaluating to `b`. */
abstract predicate checks(Instruction instr, boolean b);
predicate checksInstr(Instruction instr, boolean b) { none() }
/** Override this predicate to hold if this guard validates `expr` upon evaluating to `b`. */
predicate checks(Expr e, boolean b) { none() }
/** Gets a node guarded by this guard. */
final Node getAGuardedNode() {
exists(ValueNumber value, boolean edge |
(
this.checksInstr(value.getAnInstruction(), edge)
or
this.checks(value.getAnInstruction().getConvertedResultExpression(), edge)
) and
result.asInstruction() = value.getAnInstruction() and
this.checks(value.getAnInstruction(), edge) and
this.controls(result.asInstruction().getBlock(), edge)
)
}

View File

@@ -508,7 +508,8 @@ static_asserts(
unique int id: @static_assert,
int condition : @expr ref,
string message : string ref,
int location: @location_default ref
int location: @location_default ref,
int enclosing : @element ref
);
// each function has an ordered list of parameters

File diff suppressed because it is too large Load Diff

View File

@@ -10,7 +10,7 @@ import semmle.code.cpp.ir.IR
class TestBarrierGuard extends DataFlow::BarrierGuard {
TestBarrierGuard() { this.(CallInstruction).getStaticCallTarget().getName() = "guarded" }
override predicate checks(Instruction checked, boolean isTrue) {
override predicate checksInstr(Instruction checked, boolean isTrue) {
checked = this.(CallInstruction).getPositionalArgument(0) and
isTrue = true
}

View File

@@ -112,7 +112,7 @@ void test1()
{
char buffer[256] = {0};
sink(mysprintf(buffer, 256, "%s", string::source()));
sink(buffer); // tainted [NOT DETECTED - implement UserDefinedFormattingFunction.getOutputParameterIndex()]
sink(buffer); // tainted
}
{

View File

@@ -206,6 +206,8 @@
| format.cpp:113:21:113:24 | {...} | format.cpp:115:8:115:13 | buffer | |
| format.cpp:113:23:113:23 | 0 | format.cpp:113:21:113:24 | {...} | TAINT |
| format.cpp:114:18:114:23 | ref arg buffer | format.cpp:115:8:115:13 | buffer | |
| format.cpp:114:31:114:34 | %s | format.cpp:114:18:114:23 | ref arg buffer | TAINT |
| format.cpp:114:37:114:50 | call to source | format.cpp:114:18:114:23 | ref arg buffer | TAINT |
| format.cpp:119:10:119:11 | 0 | format.cpp:120:29:120:29 | i | |
| format.cpp:119:10:119:11 | 0 | format.cpp:121:8:121:8 | i | |
| format.cpp:120:28:120:29 | ref arg & ... | format.cpp:120:29:120:29 | i [inner post update] | |

View File

@@ -22,6 +22,7 @@
| format.cpp:100:8:100:13 | buffer | format.cpp:99:30:99:43 | call to source |
| format.cpp:105:8:105:13 | buffer | format.cpp:104:31:104:45 | call to source |
| format.cpp:110:8:110:14 | wbuffer | format.cpp:109:38:109:52 | call to source |
| format.cpp:115:8:115:13 | buffer | format.cpp:114:37:114:50 | call to source |
| format.cpp:157:7:157:22 | access to array | format.cpp:147:12:147:25 | call to source |
| format.cpp:158:7:158:27 | ... + ... | format.cpp:148:16:148:30 | call to source |
| movableclass.cpp:44:8:44:9 | s1 | movableclass.cpp:39:21:39:26 | call to source |

View File

@@ -22,6 +22,7 @@
| format.cpp:100:8:100:13 | format.cpp:99:30:99:43 | AST only |
| format.cpp:105:8:105:13 | format.cpp:104:31:104:45 | AST only |
| format.cpp:110:8:110:14 | format.cpp:109:38:109:52 | AST only |
| format.cpp:115:8:115:13 | format.cpp:114:37:114:50 | AST only |
| movableclass.cpp:44:8:44:9 | movableclass.cpp:39:21:39:26 | AST only |
| movableclass.cpp:45:8:45:9 | movableclass.cpp:40:23:40:28 | AST only |
| movableclass.cpp:46:8:46:9 | movableclass.cpp:42:8:42:13 | AST only |

View File

@@ -2,3 +2,4 @@
| tests.cpp:259:2:259:8 | call to sprintf | This 'call to sprintf' operation requires 17 bytes but the destination is only 10 bytes. |
| tests.cpp:272:2:272:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
| tests.cpp:273:2:273:8 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |
| tests.cpp:308:3:308:9 | call to sprintf | This 'call to sprintf' operation requires 9 bytes but the destination is only 8 bytes. |

View File

@@ -289,3 +289,22 @@ void test5(va_list args, float f)
vsprintf(buffer4, "123", args); // GOOD
vsprintf(buffer4, "1234", args); // BAD: buffer overflow [NOT DETECTED]
}
namespace custom_sprintf_impl {
int sprintf(char *buf, const char *format, ...)
{
__builtin_va_list args;
int i;
__builtin_va_start(args, format);
i = vsprintf(buf, format, args);
__builtin_va_end(args);
return i;
}
void regression_test1()
{
char buffer8[8];
sprintf(buffer8, "12345678"); // BAD: potential buffer overflow
}
}

File diff suppressed because it is too large Load Diff

File diff suppressed because it is too large Load Diff

View File

@@ -0,0 +1,21 @@
class StaticAssert extends @static_assert {
string toString() { none() }
}
class Expr extends @expr {
string toString() { none() }
}
class Location extends @location_default {
string toString() { none() }
}
class NameSpace extends @namespace {
string toString() { none() }
}
from StaticAssert sa, Expr condition, string message, Location loc, NameSpace ns
where
static_asserts(sa, condition, message, loc) and
namespaces(ns, "")
select sa, condition, message, loc, ns

View File

@@ -0,0 +1,4 @@
description: Give static_assert's an enclosing element
compatibility: partial
static_asserts.rel: run static_asserts.qlo

9
csharp/.vscode/extensions.json vendored Normal file
View File

@@ -0,0 +1,9 @@
{
"recommendations": [
"github.vscode-codeql",
"ms-dotnettools.csharp",
"formulahendry.dotnet-test-explorer",
"hbenl.vscode-test-explorer"
],
"unwantedRecommendations": []
}

7
csharp/.vscode/settings.json vendored Normal file
View File

@@ -0,0 +1,7 @@
{
"dotnet-test-explorer.enableTelemetry": false,
"dotnet-test-explorer.testProjectPath": "**/*Tests.@(csproj|vbproj|fsproj)",
"dotnet-test-explorer.testArguments": "/property:GenerateTargetFrameworkAttribute=false",
"csharp.supressBuildAssetsNotification": true,
"csharp.suppressDotnetRestoreNotification": true
}

53
csharp/.vscode/tasks.json vendored Normal file
View File

@@ -0,0 +1,53 @@
{
"version": "2.0.0",
"tasks": [
{
"label": "dotnet build",
"command": "dotnet",
"type": "shell",
"args": [
"build",
// Ask dotnet build to generate full paths for file names.
"/property:GenerateFullPaths=true",
// Do not generate summary otherwise it leads to duplicate errors in Problems panel
"/consoleloggerparameters:NoSummary"
],
"group": "build",
"presentation": {
"reveal": "always"
},
"problemMatcher": "$msCompile"
},
{
"label": "dotnet rebuild",
"command": "dotnet",
"type": "shell",
"args": [
"build",
"--no-incremental",
"/property:GenerateFullPaths=true",
"/consoleloggerparameters:NoSummary"
],
"group": "build",
"presentation": {
"reveal": "always"
},
"problemMatcher": "$msCompile"
},
{
"label": "dotnet test",
"command": "dotnet",
"type": "shell",
"args": [
"test",
"/property:GenerateFullPaths=true",
"/consoleloggerparameters:NoSummary"
],
"group": "test",
"presentation": {
"reveal": "always"
},
"problemMatcher": "$msCompile"
}
]
}

View File

@@ -19,15 +19,15 @@ namespace Semmle.Autobuild.CSharp.Tests
/// <summary>
/// List of strings passed to FileDelete.
/// </summary>
public IList<string> FileDeleteIn = new List<string>();
public readonly IList<string> FileDeleteIn = new List<string>();
void IBuildActions.FileDelete(string file)
{
FileDeleteIn.Add(file);
}
public IList<string> FileExistsIn = new List<string>();
public IDictionary<string, bool> FileExists = new Dictionary<string, bool>();
public readonly IList<string> FileExistsIn = new List<string>();
public readonly IDictionary<string, bool> FileExists = new Dictionary<string, bool>();
bool IBuildActions.FileExists(string file)
{
@@ -39,10 +39,10 @@ namespace Semmle.Autobuild.CSharp.Tests
throw new ArgumentException("Missing FileExists " + file);
}
public IList<string> RunProcessIn = new List<string>();
public IDictionary<string, int> RunProcess = new Dictionary<string, int>();
public IDictionary<string, string> RunProcessOut = new Dictionary<string, string>();
public IDictionary<string, string> RunProcessWorkingDirectory = new Dictionary<string, string>();
public readonly IList<string> RunProcessIn = new List<string>();
public readonly IDictionary<string, int> RunProcess = new Dictionary<string, int>();
public readonly IDictionary<string, string> RunProcessOut = new Dictionary<string, string>();
public readonly IDictionary<string, string> RunProcessWorkingDirectory = new Dictionary<string, string>();
int IBuildActions.RunProcess(string cmd, string args, string? workingDirectory, IDictionary<string, string>? env, out IList<string> stdOut)
{
@@ -72,14 +72,14 @@ namespace Semmle.Autobuild.CSharp.Tests
throw new ArgumentException("Missing RunProcess " + pattern);
}
public IList<string> DirectoryDeleteIn = new List<string>();
public readonly IList<string> DirectoryDeleteIn = new List<string>();
void IBuildActions.DirectoryDelete(string dir, bool recursive)
{
DirectoryDeleteIn.Add(dir);
}
public IDictionary<string, bool> DirectoryExists = new Dictionary<string, bool>();
public readonly IDictionary<string, bool> DirectoryExists = new Dictionary<string, bool>();
bool IBuildActions.DirectoryExists(string dir)
{
@@ -88,7 +88,7 @@ namespace Semmle.Autobuild.CSharp.Tests
throw new ArgumentException("Missing DirectoryExists " + dir);
}
public IDictionary<string, string?> GetEnvironmentVariable = new Dictionary<string, string?>();
public readonly IDictionary<string, string?> GetEnvironmentVariable = new Dictionary<string, string?>();
string? IBuildActions.GetEnvironmentVariable(string name)
{
@@ -104,7 +104,7 @@ namespace Semmle.Autobuild.CSharp.Tests
return GetCurrentDirectory;
}
public IDictionary<string, string> EnumerateFiles = new Dictionary<string, string>();
public readonly IDictionary<string, string> EnumerateFiles = new Dictionary<string, string>();
IEnumerable<string> IBuildActions.EnumerateFiles(string dir)
{
@@ -113,7 +113,7 @@ namespace Semmle.Autobuild.CSharp.Tests
throw new ArgumentException("Missing EnumerateFiles " + dir);
}
public IDictionary<string, string> EnumerateDirectories = new Dictionary<string, string>();
public readonly IDictionary<string, string> EnumerateDirectories = new Dictionary<string, string>();
IEnumerable<string> IBuildActions.EnumerateDirectories(string dir)
{
@@ -137,7 +137,8 @@ namespace Semmle.Autobuild.CSharp.Tests
{
}
public IDictionary<string, XmlDocument> LoadXml = new Dictionary<string, XmlDocument>();
public readonly IDictionary<string, XmlDocument> LoadXml = new Dictionary<string, XmlDocument>();
XmlDocument IBuildActions.LoadXml(string filename)
{
if (LoadXml.TryGetValue(filename, out var xml))
@@ -178,10 +179,10 @@ namespace Semmle.Autobuild.CSharp.Tests
public class BuildScriptTests
{
TestActions Actions = new TestActions();
readonly TestActions Actions = new TestActions();
// Records the arguments passed to StartCallback.
IList<string> StartCallbackIn = new List<string>();
readonly IList<string> StartCallbackIn = new List<string>();
void StartCallback(string s, bool silent)
{
@@ -189,8 +190,8 @@ namespace Semmle.Autobuild.CSharp.Tests
}
// Records the arguments passed to EndCallback
IList<string> EndCallbackIn = new List<string>();
IList<int> EndCallbackReturn = new List<int>();
readonly IList<string> EndCallbackIn = new List<string>();
readonly IList<int> EndCallbackReturn = new List<int>();
void EndCallback(int ret, string s, bool silent)
{

View File

@@ -19,6 +19,12 @@ namespace Semmle.Autobuild.Shared
BuildScript Analyse(Autobuilder builder, bool auto);
}
/// <summary>
/// A delegate used to wrap a build script in an environment where an appropriate
/// version of .NET Core is automatically installed.
/// </summary>
public delegate BuildScript WithDotNet(Autobuilder builder, Func<IDictionary<string, string>?, BuildScript> f);
/// <summary>
/// Exception indicating that environment variables are missing or invalid.
/// </summary>

View File

@@ -11,9 +11,9 @@ namespace Semmle.Autobuild.Shared
/// </summary>
public class BuildCommandAutoRule : IBuildRule
{
private readonly Func<Autobuilder, Func<IDictionary<string, string>?, BuildScript>, BuildScript> withDotNet;
private readonly WithDotNet withDotNet;
public BuildCommandAutoRule(Func<Autobuilder, Func<IDictionary<string, string>?, BuildScript>, BuildScript> withDotNet)
public BuildCommandAutoRule(WithDotNet withDotNet)
{
this.withDotNet = withDotNet;
}

View File

@@ -8,9 +8,9 @@ namespace Semmle.Autobuild.Shared
/// </summary>
public class BuildCommandRule : IBuildRule
{
private readonly Func<Autobuilder, Func<IDictionary<string, string>?, BuildScript>, BuildScript> withDotNet;
private readonly WithDotNet withDotNet;
public BuildCommandRule(Func<Autobuilder, Func<IDictionary<string, string>?, BuildScript>, BuildScript> withDotNet)
public BuildCommandRule(WithDotNet withDotNet)
{
this.withDotNet = withDotNet;
}

View File

@@ -0,0 +1,18 @@
name: "csharp"
display_name: "C#"
version: 1.22.1
column_kind: "utf16"
extra_env_vars:
DOTNET_GENERATE_ASPNET_CERTIFICATE: "false"
COR_ENABLE_PROFILING: "1"
COR_PROFILER: "{A3C70A64-7D41-4A94-A3F6-FD47D9259997}"
COR_PROFILER_PATH_64: "${env.CODEQL_EXTRACTOR_CSHARP_ROOT}/tools/${env.CODEQL_PLATFORM}/clrtracer64${env.CODEQL_PLATFORM_DLL_EXTENSION}"
CORECLR_ENABLE_PROFILING: "1"
CORECLR_PROFILER: "{A3C70A64-7D41-4A94-A3F6-FD47D9259997}"
CORECLR_PROFILER_PATH_64: "${env.CODEQL_EXTRACTOR_CSHARP_ROOT}/tools/${env.CODEQL_PLATFORM}/clrtracer64${env.CODEQL_PLATFORM_DLL_EXTENSION}"
file_types:
- name: cs
display_name: C# sources
extensions:
- .cs
legacy_qltest_extraction: true

View File

@@ -185,7 +185,7 @@ namespace Semmle.Extraction.Tests
class StringTrapEmitter : ITrapEmitter
{
string Content;
readonly string Content;
public StringTrapEmitter(string content)
{
Content = content;

View File

@@ -191,7 +191,7 @@ namespace Semmle.Extraction.Tests
[Fact]
public void ArchiveArguments()
{
var sw = new StringWriter();
using var sw = new StringWriter();
var file = Path.GetTempFileName();
try

View File

@@ -32,7 +32,7 @@ namespace Semmle.Extraction.Entities
class GeneratedLocationFactory : ICachedEntityFactory<string?, GeneratedLocation>
{
public static GeneratedLocationFactory Instance = new GeneratedLocationFactory();
public static readonly GeneratedLocationFactory Instance = new GeneratedLocationFactory();
public GeneratedLocation Create(Context cx, string? init) => new GeneratedLocation(cx);
}

View File

@@ -21,15 +21,7 @@ namespace SemmleTests.Semmle.Util
// Change directories to a directory that is in canonical form.
Directory.SetCurrentDirectory(cache.GetCanonicalPath(Path.GetTempPath()));
if (Win32.IsWindows())
{
root = @"X:\";
}
else
{
root = "/";
}
root = Win32.IsWindows() ? @"X:\" : "/";
}
void IDisposable.Dispose()
@@ -143,10 +135,10 @@ namespace SemmleTests.Semmle.Util
Assert.Equal(0, cache.CacheSize);
// The file "ABC" will fill the cache with parent directory info.
string cp = cache.GetCanonicalPath("ABC");
cache.GetCanonicalPath("ABC");
Assert.True(cache.CacheSize == 2);
cp = cache.GetCanonicalPath("def");
string cp = cache.GetCanonicalPath("def");
Assert.Equal(2, cache.CacheSize);
Assert.Equal(Path.GetFullPath("def"), cp);
}

View File

@@ -93,7 +93,7 @@ namespace SemmleTests.Semmle.Util
Assert.Equal("def", File.ReadAllText(shortPath));
}
byte[] buffer1 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
readonly byte[] buffer1 = new byte[10] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9 };
[Fact]
public void CreateShortStream()

View File

@@ -0,0 +1,3 @@
name: codeql-csharp-examples
version: 0.0.0
libraryPathDependencies: codeql-csharp

View File

@@ -6,7 +6,7 @@
* @problem.severity recommendation
* @precision high
* @id cs/inefficient-containskey
* @tag maintainability efficiency
* @tags maintainability efficiency
*/
import csharp

View File

@@ -7,3 +7,10 @@
tags contain:
- ide-contextual-queries/local-definitions
- ide-contextual-queries/local-references
- query: Metrics/Dependencies/ExternalDependencies.ql
- query: Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
- query: Metrics/Files/FLinesOfCode.ql
- query: Metrics/Files/FLinesOfCommentedCode.ql
- query: Metrics/Files/FLinesOfComment.ql
- query: Metrics/Files/FLinesOfDuplicatedCode.ql
- query: Metrics/Files/FNumberOfTests.ql

View File

@@ -1,7 +1,7 @@
/**
* Provides classes representing individual opcodes.
*
* See ECMA-335 (http://www.ecma-international.org/publications/files/ECMA-ST/Ecma-335.pdf)
* See ECMA-335 (https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf)
* pages 32-101 for a detailed explanation of these instructions.
*/

View File

@@ -876,6 +876,8 @@ module Internal {
not e.(QualifiableExpr).isConditional()
or
e instanceof SuppressNullableWarningExpr
or
e.stripCasts().getType() = any(ValueType t | not t instanceof NullableType)
}
/** Holds if expression `e2` is a non-`null` value whenever `e1` is. */

View File

@@ -197,42 +197,44 @@ private predicate isNullDefaultArgument(Ssa::ExplicitDefinition def, AlwaysNullE
/** Holds if `def` is an SSA definition that may be `null`. */
private predicate defMaybeNull(Ssa::Definition def, string msg, Element reason) {
// A variable compared to `null` might be `null`
exists(G::DereferenceableExpr de | de = def.getARead() |
reason = de.getANullCheck(_, true) and
msg = "as suggested by $@ null check" and
not de = any(Ssa::PseudoDefinition pdef).getARead() and
strictcount(Element e | e = any(Ssa::Definition def0 | de = def0.getARead()).getElement()) = 1 and
not nonNullDef(def) and
// Don't use a check as reason if there is a `null` assignment
// or argument
not def.(Ssa::ExplicitDefinition).getADefinition().getSource() instanceof MaybeNullExpr and
not isMaybeNullArgument(def, _)
)
or
// A parameter might be `null` if there is a `null` argument somewhere
isMaybeNullArgument(def, reason) and
not nonNullDef(def) and
(
if reason instanceof AlwaysNullExpr
then msg = "because of $@ null argument"
else msg = "because of $@ potential null argument"
)
or
isNullDefaultArgument(def, reason) and msg = "because the parameter has a null default value"
or
// If the source of a variable is `null` then the variable may be `null`
exists(AssignableDefinition adef | adef = def.(Ssa::ExplicitDefinition).getADefinition() |
adef.getSource() instanceof MaybeNullExpr and
reason = adef.getExpr() and
msg = "because of $@ assignment"
)
or
// A variable of nullable type may be null
exists(Dereference d | dereferenceAt(_, _, def, d) |
d.hasNullableType() and
not def instanceof Ssa::PseudoDefinition and
reason = def.getSourceVariable().getAssignable() and
msg = "because it has a nullable type"
// A variable compared to `null` might be `null`
exists(G::DereferenceableExpr de | de = def.getARead() |
reason = de.getANullCheck(_, true) and
msg = "as suggested by $@ null check" and
not de = any(Ssa::PseudoDefinition pdef).getARead() and
strictcount(Element e | e = any(Ssa::Definition def0 | de = def0.getARead()).getElement()) = 1 and
// Don't use a check as reason if there is a `null` assignment
// or argument
not def.(Ssa::ExplicitDefinition).getADefinition().getSource() instanceof MaybeNullExpr and
not isMaybeNullArgument(def, _)
)
or
// A parameter might be `null` if there is a `null` argument somewhere
isMaybeNullArgument(def, reason) and
(
if reason instanceof AlwaysNullExpr
then msg = "because of $@ null argument"
else msg = "because of $@ potential null argument"
)
or
isNullDefaultArgument(def, reason) and msg = "because the parameter has a null default value"
or
// If the source of a variable is `null` then the variable may be `null`
exists(AssignableDefinition adef | adef = def.(Ssa::ExplicitDefinition).getADefinition() |
adef.getSource() instanceof MaybeNullExpr and
reason = adef.getExpr() and
msg = "because of $@ assignment"
)
or
// A variable of nullable type may be null
exists(Dereference d | dereferenceAt(_, _, def, d) |
d.hasNullableType() and
not def instanceof Ssa::PseudoDefinition and
reason = def.getSourceVariable().getAssignable() and
msg = "because it has a nullable type"
)
)
}

View File

@@ -395,7 +395,7 @@ private predicate fieldOrPropertyStore(Expr e, Content c, Expr src, Expr q, bool
f.isFieldLike() and
f instanceof InstanceFieldOrProperty
or
exists(AccessPath ap |
exists(LibraryFlow::AdjustedAccessPath ap |
LibraryFlow::libraryFlowSummary(_, _, ap, _, _, _) and
ap.contains(f.getContent())
)
@@ -440,7 +440,7 @@ private predicate fieldOrPropertyRead(Expr e1, Content c, FieldOrPropertyRead e2
ret.isFieldLike() and
ret = e2.getTarget()
or
exists(AccessPath ap, Property target |
exists(LibraryFlow::AdjustedAccessPath ap, Property target |
LibraryFlow::libraryFlowSummary(_, _, _, _, ap, _) and
ap.contains(ret.getContent()) and
target.getGetter() = e2.(PropertyCall).getARuntimeTarget() and
@@ -569,8 +569,9 @@ private module Cached {
)
} or
TLibraryCodeNode(
ControlFlow::Node callCfn, CallableFlowSource source, AccessPath sourceAp,
CallableFlowSink sink, AccessPath sinkAp, boolean preservesValue, LibraryCodeNodeState state
ControlFlow::Node callCfn, CallableFlowSource source, AdjustedAccessPath sourceAp,
CallableFlowSink sink, AdjustedAccessPath sinkAp, boolean preservesValue,
LibraryCodeNodeState state
) {
libraryFlowSummary(callCfn.getElement(), source, sourceAp, sink, sinkAp, preservesValue) and
(
@@ -1451,11 +1452,12 @@ import OutNodes
module LibraryFlow {
pragma[nomagic]
private ValueOrRefType getPreciseSourceProperty0(
Call call, CallableFlowSource source, AccessPath sourceAp, Property p
Call call, CallableFlowSource source, AccessPath sourceAp, Property p, AccessPath sourceApTail
) {
exists(LibraryTypeDataFlow ltdf, Property p0 |
ltdf.callableFlow(source, sourceAp, _, _, call.getTarget().getSourceDeclaration(), _) and
sourceAp = AccessPath::property(p0) and
sourceAp.getHead().(PropertyContent).getProperty() = p0 and
sourceAp.getTail() = sourceApTail and
overridesOrImplementsSourceDecl(p, p0) and
result = source.getSourceType(call)
)
@@ -1476,18 +1478,19 @@ module LibraryFlow {
*/
pragma[nomagic]
private Property getPreciseSourceProperty(
Call call, CallableFlowSource source, AccessPath sourceAp
Call call, CallableFlowSource source, AccessPath sourceAp, AccessPath sourceApTail
) {
getPreciseSourceProperty0(call, source, sourceAp, result).hasMember(result)
getPreciseSourceProperty0(call, source, sourceAp, result, sourceApTail).hasMember(result)
}
pragma[nomagic]
private ValueOrRefType getPreciseSinkProperty0(
Call call, CallableFlowSink sink, AccessPath sinkAp, Property p
Call call, CallableFlowSink sink, AccessPath sinkAp, Property p, AccessPath sinkApTail
) {
exists(LibraryTypeDataFlow ltdf, Property p0 |
ltdf.callableFlow(_, _, sink, sinkAp, call.getTarget().getSourceDeclaration(), _) and
sinkAp = AccessPath::property(p0) and
sinkAp.getHead().(PropertyContent).getProperty() = p0 and
sinkAp.getTail() = sinkApTail and
overridesOrImplementsSourceDecl(p, p0) and
result = sink.getSinkType(call)
)
@@ -1510,8 +1513,132 @@ module LibraryFlow {
* from `IEnumerator<T>`.
*/
pragma[nomagic]
private Property getPreciseSinkProperty(Call call, CallableFlowSink sink, AccessPath sinkAp) {
getPreciseSinkProperty0(call, sink, sinkAp, result).hasMember(result)
private Property getPreciseSinkProperty(
Call call, CallableFlowSink sink, AccessPath sinkAp, AccessPath sinkApTail
) {
getPreciseSinkProperty0(call, sink, sinkAp, result, sinkApTail).hasMember(result)
}
predicate adjustSourceHead(
Call call, CallableFlowSource source, AccessPath sourceAp0, AccessPath sourceApTail,
PropertyContent p
) {
overridesOrImplementsSourceDecl(p.getProperty(),
getPreciseSourceProperty(call, source, sourceAp0, sourceApTail).getSourceDeclaration())
}
predicate adjustSinkHead(
Call call, CallableFlowSink sink, AccessPath sinkAp0, AccessPath sinkApTail, PropertyContent p
) {
p.getProperty() = getPreciseSinkProperty(call, sink, sinkAp0, sinkApTail).getSourceDeclaration()
}
private newtype TAdjustedAccessPath =
TOriginalAccessPath(AccessPath ap) or
THeadAdjustedAccessPath(PropertyContent head, AccessPath tail) {
adjustSourceHead(_, _, _, tail, head)
or
adjustSinkHead(_, _, _, tail, head)
}
/**
* An access path used in a library-code flow-summary, where the head of the path
* may have been adjusted. For example, in
*
* ```csharp
* var list = new List<string>();
* list.Add("taint");
* var enumerator = list.getEnumerator();
* ```
*
* the step from `list` to `list.getEnumerator()`, which may be modeled as a
* read of a collection element followed by a store into the `Current`
* property, can be strengthened to be a store into the `Current` property
* from `List<T>.Enumerator`, rather than the generic `Current` property
* from `IEnumerator<T>`.
*/
abstract class AdjustedAccessPath extends TAdjustedAccessPath {
/** Gets the head of this access path, if any. */
abstract Content getHead();
/** Gets the tail of this access path, if any. */
abstract AdjustedAccessPath getTail();
/** Gets the length of this access path. */
abstract int length();
/** Gets the access path obtained by dropping the first `i` elements, if any. */
abstract AdjustedAccessPath drop(int i);
/** Holds if this access path contains content `c`. */
predicate contains(Content c) { c = this.drop(_).getHead() }
/** Gets a textual representation of this access path. */
string toString() {
exists(Content head, AdjustedAccessPath tail |
head = this.getHead() and
tail = this.getTail() and
if tail.length() = 0 then result = head.toString() else result = head + ", " + tail
)
or
this.length() = 0 and
result = "<empty>"
}
}
private class OriginalAccessPath extends AdjustedAccessPath, TOriginalAccessPath {
private AccessPath ap;
OriginalAccessPath() { this = TOriginalAccessPath(ap) }
override Content getHead() { result = ap.getHead() }
override AdjustedAccessPath getTail() { result = TOriginalAccessPath(ap.getTail()) }
override int length() { result = ap.length() }
override AdjustedAccessPath drop(int i) { result = TOriginalAccessPath(ap.drop(i)) }
}
private class HeadAdjustedAccessPath extends AdjustedAccessPath, THeadAdjustedAccessPath {
private PropertyContent head;
private AccessPath tail;
HeadAdjustedAccessPath() { this = THeadAdjustedAccessPath(head, tail) }
override Content getHead() { result = head }
override AdjustedAccessPath getTail() { result = TOriginalAccessPath(tail) }
override int length() { result = 1 + tail.length() }
override AdjustedAccessPath drop(int i) {
i = 0 and result = this
or
result = TOriginalAccessPath(tail.drop(i - 1))
}
}
module AdjustedAccessPath {
AdjustedAccessPath empty() { result.length() = 0 }
AdjustedAccessPath singleton(Content c) { result.getHead() = c and result.length() = 1 }
}
pragma[nomagic]
private predicate callableFlow(
CallableFlowSource source, AccessPath sourceAp, boolean adjustSourceAp, CallableFlowSink sink,
AccessPath sinkAp, boolean adjustSinkAp, SourceDeclarationCallable c, boolean preservesValue
) {
any(LibraryTypeDataFlow ltdf).callableFlow(source, sourceAp, sink, sinkAp, c, preservesValue) and
(
if sourceAp.getHead() instanceof PropertyContent
then adjustSourceAp = true
else adjustSourceAp = false
) and
if sinkAp.getHead() instanceof PropertyContent
then adjustSinkAp = true
else adjustSinkAp = false
}
/**
@@ -1524,33 +1651,38 @@ module LibraryFlow {
*/
pragma[nomagic]
predicate libraryFlowSummary(
Call call, CallableFlowSource source, AccessPath sourceAp, CallableFlowSink sink,
AccessPath sinkAp, boolean preservesValue
Call call, CallableFlowSource source, AdjustedAccessPath sourceAp, CallableFlowSink sink,
AdjustedAccessPath sinkAp, boolean preservesValue
) {
exists(LibraryTypeDataFlow ltdf, SourceDeclarationCallable c |
c = call.getTarget().getSourceDeclaration()
|
ltdf.callableFlow(source, sink, c, preservesValue) and
sourceAp = AccessPath::empty() and
sinkAp = AccessPath::empty()
exists(SourceDeclarationCallable c | c = call.getTarget().getSourceDeclaration() |
any(LibraryTypeDataFlow ltdf).callableFlow(source, sink, c, preservesValue) and
sourceAp = TOriginalAccessPath(AccessPath::empty()) and
sinkAp = TOriginalAccessPath(AccessPath::empty())
or
exists(AccessPath sourceAp0, AccessPath sinkAp0 |
ltdf.callableFlow(source, sourceAp0, sink, sinkAp0, c, preservesValue) and
exists(
AccessPath sourceAp0, boolean adjustSourceAp, AccessPath sinkAp0, boolean adjustSinkAp
|
callableFlow(source, sourceAp0, adjustSourceAp, sink, sinkAp0, adjustSinkAp, c,
preservesValue) and
(
not sourceAp0 = AccessPath::property(_) and
sourceAp = sourceAp0
adjustSourceAp = false and
sourceAp = TOriginalAccessPath(sourceAp0)
or
exists(Property p |
overridesOrImplementsSourceDecl(p,
getPreciseSourceProperty(call, source, sourceAp0).getSourceDeclaration()) and
sourceAp = AccessPath::property(p)
adjustSourceAp = true and
exists(PropertyContent p, AccessPath sourceApTail |
adjustSourceHead(call, source, sourceAp0, sourceApTail, p) and
sourceAp = THeadAdjustedAccessPath(p, sourceApTail)
)
) and
(
not sinkAp0 = AccessPath::property(_) and
sinkAp = sinkAp0
adjustSinkAp = false and
sinkAp = TOriginalAccessPath(sinkAp0)
or
sinkAp = AccessPath::property(getPreciseSinkProperty(call, sink, sinkAp0))
adjustSinkAp = true and
exists(PropertyContent p, AccessPath sinkApTail |
adjustSinkHead(call, sink, sinkAp0, sinkApTail, p) and
sinkAp = THeadAdjustedAccessPath(p, sinkApTail)
)
)
)
)
@@ -1606,8 +1738,8 @@ module LibraryFlow {
}
newtype TLibraryCodeNodeState =
TLibraryCodeNodeAfterReadState(AccessPath ap) { ap.length() > 0 } or
TLibraryCodeNodeBeforeStoreState(AccessPath ap) { ap.length() > 0 }
TLibraryCodeNodeAfterReadState(AdjustedAccessPath ap) { ap.length() > 0 } or
TLibraryCodeNodeBeforeStoreState(AdjustedAccessPath ap) { ap.length() > 0 }
/**
* A state used to break up (complex) flow summaries for library code into atomic
@@ -1626,12 +1758,12 @@ module LibraryFlow {
*/
class LibraryCodeNodeState extends TLibraryCodeNodeState {
string toString() {
exists(AccessPath ap |
exists(AdjustedAccessPath ap |
this = TLibraryCodeNodeAfterReadState(ap) and
result = "after read: " + ap
)
or
exists(AccessPath ap |
exists(AdjustedAccessPath ap |
this = TLibraryCodeNodeBeforeStoreState(ap) and
result = "before store: " + ap
)
@@ -1639,12 +1771,12 @@ module LibraryFlow {
/** Holds if this state represents the state after the last read. */
predicate isLastReadState() {
this = TLibraryCodeNodeAfterReadState(any(AccessPath ap | ap.length() = 1))
this = TLibraryCodeNodeAfterReadState(AdjustedAccessPath::singleton(_))
}
/** Holds if this state represents the state before the first store. */
predicate isFirstStoreState() {
this = TLibraryCodeNodeBeforeStoreState(any(AccessPath ap | ap.length() = 1))
this = TLibraryCodeNodeBeforeStoreState(AdjustedAccessPath::singleton(_))
}
}
@@ -1726,21 +1858,21 @@ module LibraryFlow {
*/
predicate localStepLibrary(Node pred, Node succ, boolean preservesValue) {
exists(
ControlFlow::Node callCfn, CallableFlowSource source, AccessPath sourceAp,
CallableFlowSink sink, AccessPath sinkAp
ControlFlow::Node callCfn, CallableFlowSource source, AdjustedAccessPath sourceAp,
CallableFlowSink sink, AdjustedAccessPath sinkAp
|
libraryFlowSummary(callCfn.getElement(), source, sourceAp, sink, sinkAp, preservesValue)
|
// Simple flow summary without reads or stores
sourceAp = AccessPath::empty() and
sinkAp = AccessPath::empty() and
sourceAp = AdjustedAccessPath::empty() and
sinkAp = AdjustedAccessPath::empty() and
entry(pred, callCfn, source) and
exit(succ, callCfn, sink)
or
// Entry step for a complex summary with no reads and (1) multiple stores, or
// (2) at least one store and non-value-preservation
exists(LibraryCodeNodeState succState |
sourceAp.length() = 0 and
sourceAp = AdjustedAccessPath::empty() and
entry(pred, callCfn, source) and
succState.isFirstStoreState() and
succ = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, succState)
@@ -1749,7 +1881,7 @@ module LibraryFlow {
// Exit step for a complex summary with no stores and (1) multiple reads, or
// (2) at least one read and non-value-preservation
exists(LibraryCodeNodeState predState |
sinkAp.length() = 0 and
sinkAp = AdjustedAccessPath::empty() and
predState.isLastReadState() and
pred = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, predState) and
exit(succ, callCfn, sink)
@@ -1758,8 +1890,8 @@ module LibraryFlow {
or
// Internal step for complex flow summaries with both reads and writes
exists(
ControlFlow::Node callCfn, CallableFlowSource source, AccessPath sourceAp,
CallableFlowSink sink, AccessPath sinkAp, LibraryCodeNodeState predState,
ControlFlow::Node callCfn, CallableFlowSource source, AdjustedAccessPath sourceAp,
CallableFlowSink sink, AdjustedAccessPath sinkAp, LibraryCodeNodeState predState,
LibraryCodeNodeState succState
|
predState.isLastReadState() and
@@ -1775,8 +1907,8 @@ module LibraryFlow {
*/
predicate setterLibrary(Node pred, Content c, Node succ, boolean preservesValue) {
exists(ControlFlow::Node callCfn, CallableFlowSource source, CallableFlowSink sink |
libraryFlowSummary(callCfn.getElement(), source, AccessPath::empty(), sink,
AccessPath::singleton(c), preservesValue)
libraryFlowSummary(callCfn.getElement(), source, AdjustedAccessPath::empty(), sink,
AdjustedAccessPath::singleton(c), preservesValue)
|
entry(pred, callCfn, source) and
exit(succ, callCfn, sink)
@@ -1790,9 +1922,9 @@ module LibraryFlow {
predicate storeStepLibrary(Node pred, Content c, Node succ) {
// Complex flow summary
exists(
ControlFlow::Node callCfn, CallableFlowSource source, AccessPath sourceAp,
CallableFlowSink sink, AccessPath sinkAp, boolean preservesValue,
LibraryCodeNodeState predState, AccessPath ap
ControlFlow::Node callCfn, CallableFlowSource source, AdjustedAccessPath sourceAp,
CallableFlowSink sink, AdjustedAccessPath sinkAp, boolean preservesValue,
LibraryCodeNodeState predState, AdjustedAccessPath ap
|
predState = TLibraryCodeNodeBeforeStoreState(ap) and
pred = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, predState) and
@@ -1800,7 +1932,8 @@ module LibraryFlow {
|
// More stores needed
exists(LibraryCodeNodeState succState |
succState = TLibraryCodeNodeBeforeStoreState(any(AccessPath succAp | succAp.getTail() = ap)) and
succState =
TLibraryCodeNodeBeforeStoreState(any(AdjustedAccessPath succAp | succAp.getTail() = ap)) and
succ = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, succState)
)
or
@@ -1819,8 +1952,8 @@ module LibraryFlow {
*/
predicate getterLibrary(Node pred, Content c, Node succ, boolean preservesValue) {
exists(ControlFlow::Node callCfn, CallableFlowSource source, CallableFlowSink sink |
libraryFlowSummary(callCfn.getElement(), source, AccessPath::singleton(c), sink,
AccessPath::empty(), preservesValue) and
libraryFlowSummary(callCfn.getElement(), source, AdjustedAccessPath::singleton(c), sink,
AdjustedAccessPath::empty(), preservesValue) and
entry(pred, callCfn, source) and
exit(succ, callCfn, sink)
)
@@ -1833,9 +1966,9 @@ module LibraryFlow {
predicate readStepLibrary(Node pred, Content c, Node succ) {
// Complex flow summary
exists(
ControlFlow::Node callCfn, CallableFlowSource source, AccessPath sourceAp,
CallableFlowSink sink, AccessPath sinkAp, boolean preservesValue,
LibraryCodeNodeState succState, AccessPath ap
ControlFlow::Node callCfn, CallableFlowSource source, AdjustedAccessPath sourceAp,
CallableFlowSink sink, AdjustedAccessPath sinkAp, boolean preservesValue,
LibraryCodeNodeState succState, AdjustedAccessPath ap
|
succState = TLibraryCodeNodeAfterReadState(ap) and
succ = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, succState) and
@@ -1846,7 +1979,7 @@ module LibraryFlow {
entry(pred, callCfn, source)
or
// Subsequent reads
exists(LibraryCodeNodeState predState, AccessPath predAp |
exists(LibraryCodeNodeState predState, AdjustedAccessPath predAp |
predState = TLibraryCodeNodeAfterReadState(predAp) and
predAp.getTail() = ap and
pred = TLibraryCodeNode(callCfn, source, sourceAp, sink, sinkAp, preservesValue, predState)
@@ -1886,9 +2019,9 @@ private DataFlowType getContentType(Content c) {
class LibraryCodeNode extends NodeImpl, TLibraryCodeNode {
private ControlFlow::Node callCfn;
private CallableFlowSource source;
private AccessPath sourceAp;
private LibraryFlow::AdjustedAccessPath sourceAp;
private CallableFlowSink sink;
private AccessPath sinkAp;
private LibraryFlow::AdjustedAccessPath sinkAp;
private boolean preservesValue;
private LibraryFlow::LibraryCodeNodeState state;
@@ -1899,7 +2032,7 @@ class LibraryCodeNode extends NodeImpl, TLibraryCodeNode {
override Callable getEnclosingCallableImpl() { result = callCfn.getEnclosingCallable() }
override DataFlowType getDataFlowType() {
exists(AccessPath ap |
exists(LibraryFlow::AdjustedAccessPath ap |
state = LibraryFlow::TLibraryCodeNodeAfterReadState(ap) and
if sinkAp.length() = 0 and state.isLastReadState() and preservesValue = true
then result = Gvn::getGlobalValueNumber(sink.getSinkType(callCfn.getElement()))

View File

@@ -8,7 +8,10 @@ alwaysNull
| dataflow.cs:80:21:80:44 | access to property NullProperty |
| dataflow.cs:89:31:89:44 | call to method NullFunction |
alwaysNotNull
| dataflow.cs:71:13:71:20 | access to local variable nonNull1 |
| dataflow.cs:71:13:71:35 | Int32 nonNull1 = ... |
| dataflow.cs:71:24:71:35 | default(...) |
| dataflow.cs:71:32:71:34 | access to type Int32 |
| dataflow.cs:72:27:72:30 | this access |
| dataflow.cs:72:27:72:40 | call to method GetType |
| dataflow.cs:73:30:73:33 | true |
@@ -25,6 +28,7 @@ alwaysNotNull
| dataflow.cs:85:24:85:30 | access to local variable nonNull |
| dataflow.cs:85:24:85:55 | call to method ReturnsNonNullIndirect |
| dataflow.cs:86:24:86:30 | access to local variable nonNull |
| dataflow.cs:89:24:89:27 | access to field cond |
| dataflow.cs:89:24:89:27 | this access |
| dataflow.cs:89:31:89:44 | this access |
| dataflow.cs:89:48:89:51 | this access |

View File

@@ -385,6 +385,32 @@ public class E
return true;
return e1.Long == e2.Long; // GOOD (false positive)
}
int Ex38(int? i)
{
i ??= 0;
return i.Value; // GOOD
}
System.Drawing.Color Ex39(System.Drawing.Color? color)
{
color ??= System.Drawing.Color.White;
return color.Value; // GOOD
}
int Ex40()
{
int? i = null;
i ??= null;
return i.Value; // BAD (always)
}
int Ex41()
{
int? i = 1;
i ??= null;
return i.Value; // GOOD
}
}
public static class Extensions
@@ -393,4 +419,4 @@ public static class Extensions
public static int M2(this string s) => s.Length;
}
// semmle-extractor-options: /r:System.Linq.dll
// semmle-extractor-options: /r:System.Linq.dll /r:System.Drawing.Primitives.dll

View File

@@ -1115,12 +1115,14 @@
| E.cs:176:13:176:14 | access to local variable b2 | true | E.cs:175:19:175:42 | ... ? ... : ... | true |
| E.cs:176:13:176:22 | ... == ... | false | E.cs:176:13:176:14 | (...) ... | non-null |
| E.cs:176:13:176:22 | ... == ... | true | E.cs:176:13:176:14 | (...) ... | null |
| E.cs:176:13:176:22 | ... == ... | true | E.cs:176:19:176:22 | null | non-null |
| E.cs:180:13:180:23 | ... == ... | false | E.cs:180:13:180:15 | access to parameter obj | non-null |
| E.cs:180:13:180:23 | ... == ... | true | E.cs:180:13:180:15 | access to parameter obj | null |
| E.cs:184:13:184:14 | (...) ... | non-null | E.cs:184:13:184:14 | access to parameter b1 | non-null |
| E.cs:184:13:184:14 | (...) ... | null | E.cs:184:13:184:14 | access to parameter b1 | null |
| E.cs:184:13:184:22 | ... == ... | false | E.cs:184:13:184:14 | (...) ... | non-null |
| E.cs:184:13:184:22 | ... == ... | true | E.cs:184:13:184:14 | (...) ... | null |
| E.cs:184:13:184:22 | ... == ... | true | E.cs:184:19:184:22 | null | non-null |
| E.cs:193:19:193:29 | call to method ToString | non-null | E.cs:193:17:193:17 | access to parameter o | non-null |
| E.cs:198:17:198:29 | ... ? ... : ... | non-null | E.cs:198:17:198:17 | access to parameter b | false |
| E.cs:198:17:198:29 | ... ? ... : ... | non-null | E.cs:198:28:198:29 | "" | non-null |
@@ -1256,6 +1258,26 @@
| E.cs:384:13:384:36 | ... && ... | true | E.cs:384:27:384:36 | ... == ... | true |
| E.cs:384:27:384:36 | ... == ... | false | E.cs:384:27:384:28 | access to parameter e2 | non-null |
| E.cs:384:27:384:36 | ... == ... | true | E.cs:384:27:384:28 | access to parameter e2 | null |
| E.cs:404:9:404:9 | access to local variable i | non-null | E.cs:403:18:403:21 | null | non-null |
| E.cs:404:9:404:9 | access to local variable i | null | E.cs:403:18:403:21 | null | null |
| E.cs:404:9:404:18 | ... = ... | non-null | E.cs:404:9:404:9 | access to local variable i | non-null |
| E.cs:404:9:404:18 | ... = ... | non-null | E.cs:404:9:404:18 | ... ?? ... | non-null |
| E.cs:404:9:404:18 | ... = ... | null | E.cs:404:9:404:9 | access to local variable i | null |
| E.cs:404:9:404:18 | ... = ... | null | E.cs:404:9:404:18 | ... ?? ... | null |
| E.cs:404:9:404:18 | ... ?? ... | null | E.cs:404:9:404:9 | access to local variable i | null |
| E.cs:404:9:404:18 | ... ?? ... | null | E.cs:404:15:404:18 | null | null |
| E.cs:405:16:405:16 | access to local variable i | non-null | E.cs:404:9:404:18 | ... ?? ... | non-null |
| E.cs:405:16:405:16 | access to local variable i | null | E.cs:404:9:404:18 | ... ?? ... | null |
| E.cs:411:9:411:9 | access to local variable i | non-null | E.cs:410:18:410:18 | (...) ... | non-null |
| E.cs:411:9:411:9 | access to local variable i | null | E.cs:410:18:410:18 | (...) ... | null |
| E.cs:411:9:411:18 | ... = ... | non-null | E.cs:411:9:411:9 | access to local variable i | non-null |
| E.cs:411:9:411:18 | ... = ... | non-null | E.cs:411:9:411:18 | ... ?? ... | non-null |
| E.cs:411:9:411:18 | ... = ... | null | E.cs:411:9:411:9 | access to local variable i | null |
| E.cs:411:9:411:18 | ... = ... | null | E.cs:411:9:411:18 | ... ?? ... | null |
| E.cs:411:9:411:18 | ... ?? ... | null | E.cs:411:9:411:9 | access to local variable i | null |
| E.cs:411:9:411:18 | ... ?? ... | null | E.cs:411:15:411:18 | null | null |
| E.cs:412:16:412:16 | access to local variable i | non-null | E.cs:411:9:411:18 | ... ?? ... | non-null |
| E.cs:412:16:412:16 | access to local variable i | null | E.cs:411:9:411:18 | ... ?? ... | null |
| Forwarding.cs:9:13:9:30 | !... | false | Forwarding.cs:9:14:9:30 | call to method IsNullOrEmpty | true |
| Forwarding.cs:9:13:9:30 | !... | true | Forwarding.cs:9:14:9:30 | call to method IsNullOrEmpty | false |
| Forwarding.cs:9:14:9:14 | access to local variable s | empty | Forwarding.cs:7:20:7:23 | null | empty |

View File

@@ -36,6 +36,7 @@
| E.cs:323:13:323:14 | access to parameter s1 | Variable $@ is always null here. | E.cs:319:29:319:30 | s1 | s1 |
| E.cs:324:13:324:14 | access to parameter s2 | Variable $@ is always null here. | E.cs:319:40:319:41 | s2 | s2 |
| E.cs:331:9:331:9 | access to local variable x | Variable $@ is always null here. | E.cs:330:13:330:13 | x | x |
| E.cs:405:16:405:16 | access to local variable i | Variable $@ is always null here. | E.cs:403:14:403:14 | i | i |
| Forwarding.cs:36:31:36:31 | access to local variable s | Variable $@ is always null here. | Forwarding.cs:7:16:7:16 | s | s |
| Forwarding.cs:40:27:40:27 | access to local variable s | Variable $@ is always null here. | Forwarding.cs:7:16:7:16 | s | s |
| NullAlwaysBad.cs:9:30:9:30 | access to parameter s | Variable $@ is always null here. | NullAlwaysBad.cs:7:29:7:29 | s | s |

View File

@@ -219,10 +219,12 @@
| E.cs:175:19:175:29 | ... == ... | E.cs:175:19:175:21 | access to parameter obj | true | true |
| E.cs:176:13:176:22 | ... == ... | E.cs:176:13:176:14 | (...) ... | false | false |
| E.cs:176:13:176:22 | ... == ... | E.cs:176:13:176:14 | (...) ... | true | true |
| E.cs:176:13:176:22 | ... == ... | E.cs:176:19:176:22 | null | true | false |
| E.cs:180:13:180:23 | ... == ... | E.cs:180:13:180:15 | access to parameter obj | false | false |
| E.cs:180:13:180:23 | ... == ... | E.cs:180:13:180:15 | access to parameter obj | true | true |
| E.cs:184:13:184:22 | ... == ... | E.cs:184:13:184:14 | (...) ... | false | false |
| E.cs:184:13:184:22 | ... == ... | E.cs:184:13:184:14 | (...) ... | true | true |
| E.cs:184:13:184:22 | ... == ... | E.cs:184:19:184:22 | null | true | false |
| E.cs:193:17:193:17 | access to parameter o | E.cs:193:17:193:17 | access to parameter o | non-null | false |
| E.cs:193:17:193:17 | access to parameter o | E.cs:193:17:193:17 | access to parameter o | null | true |
| E.cs:208:13:208:23 | ... is ... | E.cs:208:13:208:13 | access to parameter s | false | true |
@@ -276,6 +278,14 @@
| E.cs:384:13:384:22 | ... == ... | E.cs:384:13:384:14 | access to parameter e1 | true | true |
| E.cs:384:27:384:36 | ... == ... | E.cs:384:27:384:28 | access to parameter e2 | false | false |
| E.cs:384:27:384:36 | ... == ... | E.cs:384:27:384:28 | access to parameter e2 | true | true |
| E.cs:391:9:391:9 | access to parameter i | E.cs:391:9:391:9 | access to parameter i | non-null | false |
| E.cs:391:9:391:9 | access to parameter i | E.cs:391:9:391:9 | access to parameter i | null | true |
| E.cs:397:9:397:13 | access to parameter color | E.cs:397:9:397:13 | access to parameter color | non-null | false |
| E.cs:397:9:397:13 | access to parameter color | E.cs:397:9:397:13 | access to parameter color | null | true |
| E.cs:404:9:404:9 | access to local variable i | E.cs:404:9:404:9 | access to local variable i | non-null | false |
| E.cs:404:9:404:9 | access to local variable i | E.cs:404:9:404:9 | access to local variable i | null | true |
| E.cs:411:9:411:9 | access to local variable i | E.cs:411:9:411:9 | access to local variable i | non-null | false |
| E.cs:411:9:411:9 | access to local variable i | E.cs:411:9:411:9 | access to local variable i | null | true |
| Forwarding.cs:9:14:9:30 | call to method IsNullOrEmpty | Forwarding.cs:9:14:9:14 | access to local variable s | false | false |
| Forwarding.cs:14:13:14:32 | call to method IsNotNullOrEmpty | Forwarding.cs:14:13:14:13 | access to local variable s | true | false |
| Forwarding.cs:19:14:19:23 | call to method IsNull | Forwarding.cs:19:14:19:14 | access to local variable s | false | false |

View File

@@ -370,6 +370,9 @@ nodes
| E.cs:384:27:384:28 | access to parameter e2 |
| E.cs:386:16:386:17 | access to parameter e1 |
| E.cs:386:27:386:28 | access to parameter e2 |
| E.cs:404:9:404:18 | SSA def(i) |
| E.cs:404:9:404:18 | SSA def(i) |
| E.cs:405:16:405:16 | access to local variable i |
| Forwarding.cs:7:16:7:23 | SSA def(s) |
| Forwarding.cs:14:9:17:9 | if (...) ... |
| Forwarding.cs:19:9:22:9 | if (...) ... |
@@ -719,6 +722,8 @@ edges
| E.cs:384:9:385:24 | if (...) ... | E.cs:384:27:384:28 | access to parameter e2 |
| E.cs:384:9:385:24 | if (...) ... | E.cs:386:27:386:28 | access to parameter e2 |
| E.cs:384:27:384:28 | access to parameter e2 | E.cs:386:16:386:17 | access to parameter e1 |
| E.cs:404:9:404:18 | SSA def(i) | E.cs:405:16:405:16 | access to local variable i |
| E.cs:404:9:404:18 | SSA def(i) | E.cs:405:16:405:16 | access to local variable i |
| Forwarding.cs:7:16:7:23 | SSA def(s) | Forwarding.cs:14:9:17:9 | if (...) ... |
| Forwarding.cs:14:9:17:9 | if (...) ... | Forwarding.cs:19:9:22:9 | if (...) ... |
| Forwarding.cs:19:9:22:9 | if (...) ... | Forwarding.cs:24:9:27:9 | if (...) ... |

View File

@@ -0,0 +1,9 @@
@echo off
SETLOCAL EnableDelayedExpansion
rem The autobuilder is already being traced
set CODEQL_AUTOBUILDER_CSHARP_NO_INDEXING=true
type NUL && "%CODEQL_EXTRACTOR_CSHARP_ROOT%/tools/%CODEQL_PLATFORM%/Semmle.Autobuild.CSharp.exe" || exit /b %ERRORLEVEL%
ENDLOCAL

14
csharp/tools/autobuild.sh Executable file
View File

@@ -0,0 +1,14 @@
#!/bin/sh
set -eu
if [ "$CODEQL_PLATFORM" != "linux64" ] && [ "$CODEQL_PLATFORM" != "osx64" ] ; then
echo "Automatic build detection for $CODEQL_PLATFORM is not implemented."
exit 1
fi
# The autobuilder is already being traced
CODEQL_AUTOBUILDER_CSHARP_NO_INDEXING="true"
export CODEQL_AUTOBUILDER_CSHARP_NO_INDEXING
"$CODEQL_EXTRACTOR_CSHARP_ROOT/tools/$CODEQL_PLATFORM/Semmle.Autobuild.CSharp" || exit $?

View File

@@ -0,0 +1,9 @@
**/mcs.exe:
**/csc.exe:
invoke ${config_dir}/Semmle.Extraction.CSharp.Driver
prepend --compiler
prepend "${compiler}"
prepend --cil
**/mono*:
**/dotnet:
invoke ${config_dir}/extract-csharp.sh

View File

@@ -0,0 +1,16 @@
#!/bin/bash
echo extract-csharp.sh: Called with arguments: "$@"
extractor="$CODEQL_EXTRACTOR_CSHARP_ROOT/tools/$CODEQL_PLATFORM/Semmle.Extraction.CSharp.Driver"
for i in "$@"
do
shift
if [[ `basename -- "$i"` =~ csc.exe|mcs.exe|csc.dll ]]
then
echo extract-csharp.sh: exec $extractor --cil $@
exec "$extractor" --compiler $i --cil $@
fi
done
echo extract-csharp.sh: Not a compiler invocation

View File

@@ -0,0 +1,14 @@
**/mcs.exe:
**/csc.exe:
invoke ${config_dir}/Semmle.Extraction.CSharp.Driver
prepend --compiler
prepend "${compiler}"
prepend --cil
**/mono*:
**/dotnet:
invoke ${config_dir}/extract-csharp.sh
/usr/bin/codesign:
replace yes
invoke /usr/bin/env
prepend /usr/bin/codesign
trace no

View File

@@ -0,0 +1,16 @@
#!/bin/bash
echo extract-csharp.sh: Called with arguments: "$@"
extractor="$CODEQL_EXTRACTOR_CSHARP_ROOT/tools/$CODEQL_PLATFORM/Semmle.Extraction.CSharp.Driver"
for i in "$@"
do
shift
if [[ `basename -- "$i"` =~ csc.exe|mcs.exe|csc.dll ]]
then
echo extract-csharp.sh: exec $extractor --cil $@
exec "$extractor" --compiler $i --cil $@
fi
done
echo extract-csharp.sh: Not a compiler invocation

View File

@@ -0,0 +1,14 @@
@echo off
SETLOCAL EnableDelayedExpansion
type NUL && "%CODEQL_DIST%\codeql" database index-files ^
--include-extension=.config ^
--include-extension=.csproj ^
--include-extension=.props ^
--include-extension=.xml ^
--size-limit 10m ^
--language xml ^
-- ^
"%CODEQL_EXTRACTOR_CSHARP_WIP_DATABASE%"
ENDLOCAL

13
csharp/tools/pre-finalize.sh Executable file
View File

@@ -0,0 +1,13 @@
#!/bin/sh
set -eu
"$CODEQL_DIST/codeql" database index-files \
--include-extension=.config \
--include-extension=.csproj \
--include-extension=.props \
--include-extension=.xml \
--size-limit 10m \
--language xml \
-- \
"$CODEQL_EXTRACTOR_CSHARP_WIP_DATABASE"

View File

@@ -0,0 +1,9 @@
**\fakes*.exe:
**\moles*.exe:
order compiler
trace no
**\csc*.exe:
invoke ${config_dir}\Semmle.Extraction.CSharp.Driver.exe
prepend --compiler
prepend "${compiler}"
prepend --cil

Binary file not shown.

After

Width:  |  Height:  |  Size: 1.2 KiB

View File

@@ -0,0 +1,145 @@
Basic query for C and C++ code
==============================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
The query we're going to run performs a basic search of the code for ``if`` statements that are redundant, in the sense that they have an empty then branch. For example, code such as:
.. code-block:: cpp
if (error) { }
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **C/C++** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import cpp
from IfStmt ifstmt, Block block
where ifstmt.getThen() = block and
block.getNumStmt() = 0
select ifstmt, "This 'if' statement is redundant."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``ifstmt`` and is linked to the location in the source code of the project where ``ifstmt`` occurs. The second column is the alert message.
`Example query results <https://lgtm.com/query/4242591143131494898/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click a link in the ``ifstmt`` column to view the ``if`` statement in the code viewer.
The matching ``if`` statement is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+========================================================================================================================+
| ``import cpp`` | Imports the standard CodeQL libraries for C/C++. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``from IfStmt ifstmt, Block block`` | Defines the variables for the query. | We use: |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | - an ``IfStmt`` variable for ``if`` statements |
| | | - a ``Block`` variable for the statement block |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``where ifstmt.getThen() = block and block.getNumStmt() = 0`` | Defines a condition on the variables. | ``ifstmt.getThen() = block`` relates the two variables. The block must be the ``then`` branch of the ``if`` statement. |
| | | |
| | | ``block.getNumStmt() = 0`` states that the block must be empty (that is, it contains no statements). |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``select ifstmt, "This 'if' statement is redundant."`` | Defines what to report for each match. | Reports the resulting ``if`` statement with a string that explains the problem. |
| | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Browsing the results of our basic query shows that it could be improved. Among the results you are likely to find examples of ``if`` statements with an ``else`` branch, where an empty ``then`` branch does serve a purpose. For example:
.. code-block:: cpp
if (...) {
...
} else if (!strcmp(option, "-verbose") {
// nothing to do - handled earlier
} else {
error("unrecognized option");
}
In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to ignore empty ``then`` branches if the ``if`` statement has an ``else`` branch.
To exclude ``if`` statements that have an ``else`` branch:
#. Extend the ``where`` clause to include the following extra condition:
.. code-block:: ql
and not ifstmt.hasElse()
The ``where`` clause is now:
.. code-block:: ql
where ifstmt.getThen() = block and
block.getNumStmt() = 0 and
not ifstmt.hasElse()
#. Click **Run**.
There are now fewer results because ``if`` statements with an ``else`` branch are no longer reported.
`See this in the query console <https://lgtm.com/query/1899933116489579248/>`__
Further reading
---------------
.. include:: ../../reusables/cpp-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -6,6 +6,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-cpp
introduce-libraries-cpp
function-classes
expressions-types
@@ -18,7 +19,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
value-numbering-hash-cons
- `Basic C/C++ query <https://lgtm.com/help/lgtm/console/ql-cpp-basic-example>`__: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for C and C++ code <basic-query-cpp>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for C and C++ <introduce-libraries-cpp>`: When analyzing C or C++ code, you can use the large collection of classes in the CodeQL library for C and C++.

View File

@@ -0,0 +1,150 @@
Basic query for C# code
=======================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
The query we're going to run performs a basic search of the code for ``if`` statements that are redundant, in the sense that they have an empty then branch. For example, code such as:
.. code-block:: csharp
if (error) { }
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **C#** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import csharp
from IfStmt ifstmt, BlockStmt block
where ifstmt.getThen() = block and
block.isEmpty()
select ifstmt, "This 'if' statement is redundant."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``ifstmt`` and is linked to the location in the source code of the project where ``ifstmt`` occurs. The second column is the alert message.
`Example query results <https://lgtm.com/query/1214010107827821393/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click a link in the ``ifstmt`` column to view the ``if`` statement in the code viewer.
The matching ``if`` statement is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+========================================================================================================================+
| ``import csharp`` | Imports the standard CodeQL libraries for C#. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``from IfStmt ifstmt, BlockStmt block`` | Defines the variables for the query. | We use: |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | - an ``IfStmt`` variable for ``if`` statements |
| | | - a ``BlockStmt`` variable for the then block |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``where ifstmt.getThen() = block and block.isEmpty()`` | Defines a condition on the variables. | ``ifstmt.getThen() = block`` relates the two variables. The block must be the ``then`` branch of the ``if`` statement. |
| | | |
| | | ``block.isEmpty()`` states that the block must be empty (that is, it contains no statements). |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``select ifstmt, "This 'if' statement is redundant."`` | Defines what to report for each match. | Reports the resulting ``if`` statement with a string that explains the problem. |
| | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Browsing the results of our basic query shows that it could be improved. Among the results you are likely to find examples of ``if`` statements with an ``else`` branch, where an empty ``then`` branch does serve a purpose. For example:
.. code-block:: csharp
if (...)
{
...
}
else if (option == "-verbose")
{
// nothing to do - handled earlier
}
else
{
error("unrecognized option");
}
In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to ignore empty ``then`` branches if the ``if`` statement has an ``else`` branch.
To exclude ``if`` statements that have an ``else`` branch:
#. Add the following to the where clause:
.. code-block:: ql
and not exists(ifstmt.getElse())
The ``where`` clause is now:
.. code-block:: ql
where ifstmt.getThen() = block and
block.isEmpty() and
not exists(ifstmt.getElse())
#. Click **Run**.
There are now fewer results because ``if`` statements with an ``else`` branch are no longer included.
`See this in the query console <https://lgtm.com/query/6233102733683510530/>`__
Further reading
---------------
.. include:: ../../reusables/csharp-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -6,10 +6,11 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-csharp
introduce-libraries-csharp
dataflow
- `Basic C# query <https://lgtm.com/help/lgtm/console/ql-csharp-basic-example>`__: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for C# code <basic-query-csharp>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for C# <introduce-libraries-csharp>`: When you're analyzing a C# program, you can make use of the large collection of classes in the CodeQL library for C#.

View File

@@ -0,0 +1,151 @@
Basic query for Go code
=======================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
The query we're going to run searches the code for methods defined on value types that modify their receiver by writing a field:
.. code-block:: go
func (s MyStruct) valueMethod() { s.f = 1 } // method on value
This is problematic because the receiver argument is passed by value, not by reference. Consequently, valueMethod is called with a copy of the receiver object, so any changes it makes to the receiver will be invisible to the caller. To prevent this, the method should be defined on a pointer instead:
.. code-block:: go
func (s *MyStruct) pointerMethod() { s.f = 1 } // method on pointer
For further information on using methods on values or pointers in Go, see the `Go FAQ <https://golang.org/doc/faq#methods_on_values_or_pointers>`__.
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **Go** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import go
from Method m, Variable recv, Write w, Field f
where
recv = m.getReceiver() and
w.writesField(recv.getARead(), f, _) and
not recv.getType() instanceof PointerType
select w, "This update to " + f + " has no effect, because " + recv + " is not a pointer."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to ``w``, which is the location in the source code where the receiver ``recv`` is modified. The second column is the alert message.
`Example query results <https://lgtm.com/query/6221190009056970603/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click a link in the ``w`` column to view it in the code viewer.
The matching ``w`` is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+======================================================================================================================================+
| ``import go`` | Imports the standard CodeQL libraries for Go. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------+
| ``from Method m, Variable recv, Write w, Field f`` | Defines the variables for the query. | We declare: |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | - ``m`` as a variable for all methods |
| | | - a ``recv`` variable, which is the receiver of ``m`` |
| | | - ``w`` as the location in the code where the receiver is modified |
| | | - ``f`` as the field that is written when ``m`` is called |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------+
| ``where recv = m.getReceiver() and | Defines a condition on the variables. | ``recv = m.getReceiver()`` states that ``recv`` must be the receiver variable of ``m``. |
| w.writesField(recv.getARead(), f, _) and | | |
| not recv.getType() instanceof PointerType`` | | ``w.writesField(recv.getARead(), f, _)`` states that ``w`` must be a location in the code where field ``f`` of ``recv`` is modified. |
| | | We use a `'don't-care' expression <https://help.semmle.com/QL/ql-handbook/expressions.html#don-t-care-expressions>`__ _ for the |
| | | value that is written to ``f``—the actual value doesn't matter in this query. |
| | | |
| | | ``not recv.getType() instanceof PointerType`` states that ``m`` is not a pointer method. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------+
| ``select w, "This update to " + f + | Defines what to report for each match. | Reports ``w`` with a message that explains the potential problem. |
| " has no effect, because " + recv + " is not a pointer."`` | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+--------------------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Among the results generated by the first iteration of this query, you can find cases where a value method is called but the receiver variable is returned. In such cases, the change to the receiver is not invisible to the caller, so a pointer method is not required. These are false positive results and you can improve the query by adding an extra condition to remove them.
To exclude these values:
#. Extend the where clause to include the following extra condition:
.. code-block:: ql
not exists(ReturnStmt ret | ret.getExpr() = recv.getARead().asExpr())
The ``where`` clause is now:
.. code-block:: ql
where e.isPure() and
recv = m.getReceiver() and
w.writesField(recv.getARead(), f, _) and
not recv.getType() instanceof PointerType and
not exists(ReturnStmt ret | ret.getExpr() = recv.getARead().asExpr())
#. Click **Run**.
There are now fewer results because value methods that return their receiver variable are no longer reported.
`See this in the query console <https://lgtm.com/query/9110448975027954322/>`__
Further reading
---------------
.. include:: ../../reusables/go-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -554,7 +554,7 @@ fact that ``p != nil`` is ``true`` at this point:
|cfg2|
A typical use of this information would be in an analyis that looks for ``nil`` dereferences: such
A typical use of this information would be in an analysis that looks for ``nil`` dereferences: such
an analysis would be able to conclude that the field read ``p.f`` is safe because it is immediately
preceded by a condition guard node guaranteeing that ``p`` is not ``nil``.

View File

@@ -0,0 +1,122 @@
Modeling data flow in Go libraries
==================================
When analyzing a Go program, CodeQL does not examine the source code for
external packages. To track the flow of untrusted data through a library, you
can create a model of the library.
You can find existing models in the ``ql/src/semmle/go/frameworks/`` folder of the
`CodeQL for Go repository <https://github.com/github/codeql-go/tree/main/ql/src/semmle/go/frameworks>`__.
To add a new model, you should make a new file in that folder, named after the library.
Sources
-------
To mark a source of data that is controlled by an untrusted user, we
create a class extending ``UntrustedFlowSource::Range``. Inheritance and
the characteristic predicate of the class should be used to specify
exactly the dataflow node that introduces the data. Here is a short
example from ``Mux.qll``.
.. code-block:: ql
class RequestVars extends DataFlow::UntrustedFlowSource::Range, DataFlow::CallNode {
RequestVars() { this.getTarget().hasQualifiedName("github.com/gorilla/mux", "Vars") }
}
This has the effect that all calls to `the function Vars from the
package mux <http://www.gorillatoolkit.org/pkg/mux#Vars>`__ are
treated as sources of untrusted data.
Flow propagation
----------------
By default, we assume that all functions in libraries do not have
any data flow. To indicate that a particular function does have data flow,
create a class extending ``TaintTracking::FunctionModel`` (or
``DataFlow::FunctionModel`` if the untrusted user data is passed on
without being modified).
Inheritance and the characteristic predicate of the class should specify
the function. The class should also have a member predicate with the signature
``override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp)``
(or
``override predicate hasDataFlow(FunctionInput inp, FunctionOutput outp)``
if extending ``DataFlow::FunctionModel``). The body should constrain
``inp`` and ``outp``.
``FunctionInput`` is an abstract representation of the inputs to a
function. The options are:
* the receiver (``inp.isReceiver()``)
* one of the parameters (``inp.isParameter(i)``)
* one of the results (``inp.isResult(i)``, or ``inp.isResult`` if there is only one result)
Note that it may seem strange that the result of a function could be
considered as a function input, but it is needed in some cases. For
instance, the function ``bufio.NewWriter`` returns a writer ``bw`` that
buffers write operations to an underlying writer ``w``. If tainted data
is written to ``bw``, then it makes sense to propagate that taint back
to the underlying writer ``w``, which can be modeled by saying that
``bufio.NewWriter`` propagates taint from its result to its first
argument.
Similarly, ``FunctionOutput`` is an abstract representation of the
outputs to a function. The options are:
* the receiver (``outp.isReceiver()``)
* one of the parameters (``outp.isParameter(i)``)
* one of the results (``outp.isResult(i)``, or ``outp.isResult`` if there is only one result)
Here is an example from ``Gin.qll``, which has been slightly simplified.
.. code-block:: ql
private class ParamsGet extends TaintTracking::FunctionModel, Method {
ParamsGet() { this.hasQualifiedName("github.com/gin-gonic/gin", "Params", "Get") }
override predicate hasTaintFlow(FunctionInput inp, FunctionOutput outp) {
inp.isReceiver() and outp.isResult(0)
}
}
This has the effect that calls to the ``Get`` method with receiver type
``Params`` from the ``gin-gonic/gin`` package allow taint to flow from
the receiver to the first result. In other words, if ``p`` has type
``Params`` and taint can flow to it, then after the line
``x := p.Get("foo")`` taint can also flow to ``x``.
Sanitizers
----------
It is not necessary to indicate that library functions are sanitizers.
Their bodies are not analyzed, so it is assumed that data does not
flow through them.
Sinks
-----
Data-flow sinks are specified by queries rather than by library models.
However, you can use library models to indicate when functions belong to
special categories. Queries can then use these categories when specifying
sinks. Classes representing these special categories are contained in
``ql/src/semmle/go/Concepts.qll`` in the `CodeQL for Go repository
<https://github.com/github/codeql-go/blob/main/ql/src/semmle/go/Concepts.qll>`__.
``Concepts.qll`` includes classes for logger mechanisms,
HTTP response writers, HTTP redirects, and marshaling and unmarshaling
functions.
Here is a short example from ``Stdlib.qll``, which has been slightly simplified.
.. code-block:: ql
private class PrintfCall extends LoggerCall::Range, DataFlow::CallNode {
PrintfCall() { this.getTarget().hasQualifiedName("fmt", ["Print", "Printf", "Println"]) }
override DataFlow::Node getAMessageComponent() { result = this.getAnArgument() }
}
This has the effect that any call to ``Print``, ``Printf``, or
``Println`` in the package ``fmt`` is recognized as a logger call.
Any query that uses logger calls as a sink will then identify when tainted data
has been passed as an argument to ``Print``, ``Printf``, or ``Println``.

View File

@@ -6,11 +6,16 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-go
introduce-libraries-go
ast-class-reference
library-modeling-go
- `Basic Go query <https://lgtm.com/help/lgtm/console/ql-go-basic-example>`__: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for Go code <basic-query-go>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for Go <introduce-libraries-go>`: When you're analyzing a Go program, you can make use of the large collection of classes in the CodeQL library for Go.
- :doc:`Abstract syntax tree classes for working with Go programs <ast-class-reference>`: CodeQL has a large selection of classes for representing the abstract syntax tree of Go programs.
- :doc:`Modeling data flow in Go libraries <library-modeling-go>`: When analyzing a Go program, CodeQL does not examine the source code for external packages.
To track the flow of untrusted data through a library, you can create a model of the library.

View File

@@ -0,0 +1,145 @@
Basic query for Java code
=========================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
The query we're going to run performs a basic search of the code for ``if`` statements that are redundant, in the sense that they have an empty then branch. For example, code such as:
.. code-block:: java
if (error) { }
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **Java** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import java
from IfStmt ifstmt, Block block
where ifstmt.getThen() = block and
block.getNumStmt() = 0
select ifstmt, "This 'if' statement is redundant."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``ifstmt`` and is linked to the location in the source code of the project where ``ifstmt`` occurs. The second column is the alert message.
`Example query results <https://lgtm.com/query/3235645104630320782/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click a link in the ``ifstmt`` column to view the ``if`` statement in the code viewer.
The matching ``if`` statement is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+========================================================================================================================+
| ``import java`` | Imports the standard CodeQL libraries for Java. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``from IfStmt ifstmt, Block block`` | Defines the variables for the query. | We use: |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | - an ``IfStmt`` variable for ``if`` statements |
| | | - a ``Block`` variable for the then block |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``where ifstmt.getThen() = block and block.getNumStmt() = 0`` | Defines a condition on the variables. | ``ifstmt.getThen() = block`` relates the two variables. The block must be the ``then`` branch of the ``if`` statement. |
| | | |
| | | ``block.getNumStmt() = 0`` states that the block must be empty (that is, it contains no statements). |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``select ifstmt, "This 'if' statement is redundant."`` | Defines what to report for each match. | Reports the resulting ``if`` statement with a string that explains the problem. |
| | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Browsing the results of our basic query shows that it could be improved. Among the results you are likely to find examples of ``if`` statements with an ``else`` branch, where an empty ``then`` branch does serve a purpose. For example:
.. code-block:: java
if (...) {
...
} else if ("-verbose".equals(option)) {
// nothing to do - handled earlier
} else {
error("unrecognized option");
}
In this case, identifying the ``if`` statement with the empty ``then`` branch as redundant is a false positive. One solution to this is to modify the query to ignore empty ``then`` branches if the ``if`` statement has an ``else`` branch.
To exclude ``if`` statements that have an ``else`` branch:
#. Extend the where clause to include the following extra condition:
.. code-block:: ql
and not exists(ifstmt.getElse())
The ``where`` clause is now:
.. code-block:: ql
where ifstmt.getThen() = block and
block.getNumStmt() = 0 and
not exists(ifstmt.getElse())
#. Click **Run**.
There are now fewer results because ``if`` statements with an ``else`` branch are no longer included.
`See this in the query console <https://lgtm.com/query/6382189874776576029/>`__
Further reading
---------------
.. include:: ../../reusables/java-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -6,6 +6,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-java
introduce-libraries-java
dataflow
types-class-hierarchy
@@ -16,7 +17,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
source-locations
ast-class-reference
- `Basic Java query <https://lgtm.com/help/lgtm/console/ql-java-basic-example>`__: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for Java code <basic-query-java>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for Java <introduce-libraries-java>`: When analyzing Java code, you can use the large collection of classes in the CodeQL library for Java.

View File

@@ -0,0 +1,136 @@
Basic query for JavaScript code
===============================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
In JavaScript, any expression can be turned into an expression statement. While this is sometimes convenient, it can be dangerous. For example, imagine a programmer wants to assign a new value to a variable ``x`` by means of an assignment ``x = 42``. However, they accidentally type two equals signs, producing the comparison statement ``x == 42``. This is valid JavaScript, so no error is generated. The statement simply compares ``x`` to ``42``, and then discards the result of the comparison.
The query you will run finds instances of this problem. The query searches for expressions ``e`` that are pure—that is, their evaluation does not lead to any side effects—but appear as an expression statement.
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **JavaScript** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import javascript
from Expr e
where e.isPure() and
e.getParent() instanceof ExprStmt
select e, "This expression has no effect."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``e`` and is linked to the location in the source code of the project where ``e`` occurs. The second column is the alert message.
`Example query results <https://lgtm.com/query/5137013631828816943/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click one of the links in the ``e`` column to view the expression in the code viewer.
The matching statement is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+========================================================================================================================+
| ``import javascript`` | Imports the standard CodeQL libraries for JavaScript. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``from Expr e`` | Defines the variables for the query. | ``e`` is declared as a variable that ranges over expressions. |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``where e.isPure() and e.getParent() instanceof ExprStmt`` | Defines a condition on the variables. | ``e.isPure()``: The expression is side-effect-free. |
| | | |
| | | ``e.getParent() instanceof ExprStmt``: The parent of the expression is an expression statement. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``select e, "This expression has no effect."`` | Defines what to report for each match. | Report the expression with a string that explains the problem. |
| | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Browsing the results of our basic query shows that it could be improved. Among the results you are likely to find ``use strict`` directives. These are interpreted specially by modern browsers with strict mode support and so these expressions *do* have an effect.
To remove directives from the results:
#. Extend the ``where`` clause to include the following extra condition:
.. code-block:: ql
and not e.getParent() instanceof Directive
The ``where`` clause is now:
.. code-block:: ql
where e.isPure() and
e.getParent() instanceof ExprStmt and
not e.getParent() instanceof Directive
#. Click **Run**.
There are now fewer results as ``use strict`` directives are no longer reported.
The improved query finds several results on the example project including `this result <https://lgtm.com/projects/g/ajaxorg/ace/rev/ad50673d7137c09d1a5a6f0ef83633a149f9e3d1/files/lib/ace/keyboard/vim.js#L320>`__:
.. code-block:: javascript
point.bias == -1;
As written, this statement compares ``point.bias`` against ``-1`` and then discards the result. Most likely, it was instead meant to be an assignment ``point.bias = -1``.
Further reading
---------------
.. include:: ../../reusables/javascript-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -6,6 +6,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-javascript
introduce-libraries-js
introduce-libraries-ts
dataflow
@@ -14,7 +15,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
ast-class-reference
dataflow-cheat-sheet
- `Basic JavaScript query <https://lgtm.com/help/lgtm/console/ql-javascript-basic-example>`__: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for JavaScript code <basic-query-javascript>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for JavaScript <introduce-libraries-js>`: When you're analyzing a JavaScript program, you can make use of the large collection of classes in the CodeQL library for JavaScript.

View File

@@ -0,0 +1,144 @@
Basic query for Python code
===========================
Learn to write and run a simple CodeQL query using LGTM.
About the query
---------------
The query we're going to run performs a basic search of the code for ``if`` statements that are redundant, in the sense that they only include a ``pass`` statement. For example, code such as:
.. code-block:: python
if error: pass
Running the query
-----------------
#. In the main search box on LGTM.com, search for the project you want to query. For tips, see `Searching <https://lgtm.com/help/lgtm/searching>`__.
#. Click the project in the search results.
#. Click **Query this project**.
This opens the query console. (For information about using this, see `Using the query console <https://lgtm.com/help/lgtm/using-query-console>`__.)
.. pull-quote::
Note
Alternatively, you can go straight to the query console by clicking **Query console** (at the top of any page), selecting **Python** from the **Language** drop-down list, then choosing one or more projects to query from those displayed in the **Project** drop-down list.
#. Copy the following query into the text box in the query console:
.. code-block:: ql
import python
from If ifstmt, Stmt pass
where pass = ifstmt.getStmt(0) and
pass instanceof Pass
select ifstmt, "This 'if' statement is redundant."
LGTM checks whether your query compiles and, if all is well, the **Run** button changes to green to indicate that you can go ahead and run the query.
#. Click **Run**.
The name of the project you are querying, and the ID of the most recently analyzed commit to the project, are listed below the query box. To the right of this is an icon that indicates the progress of the query operation:
.. image:: ../../images/query-progress.png
:align: center
.. pull-quote::
Note
Your query is always run against the most recently analyzed commit to the selected project.
The query will take a few moments to return results. When the query completes, the results are displayed below the project name. The query results are listed in two columns, corresponding to the two expressions in the ``select`` clause of the query. The first column corresponds to the expression ``ifstmt`` and is linked to the location in the source code of the project where ``ifstmt`` occurs. The second column is the alert message.
`Example query results <https://lgtm.com/query/3592297537117272922/>`__
.. pull-quote::
Note
An ellipsis (…) at the bottom of the table indicates that the entire list is not displayed—click it to show more results.
#. If any matching code is found, click a link in the ``ifstmt`` column to view the ``if`` statement in the code viewer.
The matching ``if`` statement is highlighted with a yellow background in the code viewer. If any code in the file also matches a query from the standard query library for that language, you will see a red alert message at the appropriate point within the code.
About the query structure
~~~~~~~~~~~~~~~~~~~~~~~~~
After the initial ``import`` statement, this simple query comprises three parts that serve similar purposes to the FROM, WHERE, and SELECT parts of an SQL query.
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| Query part | Purpose | Details |
+===============================================================+===================================================================================================================+========================================================================================================================+
| ``import python`` | Imports the standard CodeQL libraries for Python. | Every query begins with one or more ``import`` statements. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``from If ifstmt, Stmt pass`` | Defines the variables for the query. | We use: |
| | Declarations are of the form: | |
| | ``<type> <variable name>`` | - an ``If`` variable for ``if`` statements |
| | | - a ``Stmt`` variable for the statement |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``where pass = ifstmt.getStmt(0) and pass instanceof Pass`` | Defines a condition on the variables. | ``pass = ifstmt.getStmt(0)``: ``pass`` is the first statement in the ``if`` statement. |
| | | |
| | | ``pass instanceof Pass``: ``pass`` must be a pass statement. |
| | | |
| | | In other words, the first statement contained in the ``if`` statement is a ``pass`` statement. |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
| ``select ifstmt, "This 'if' statement is redundant."`` | Defines what to report for each match. | Reports the resulting ``if`` statement with a string that explains the problem. |
| | | |
| | ``select`` statements for queries that are used to find instances of poor coding practice are always in the form: | |
| | ``select <program element>, "<alert message>"`` | |
+---------------------------------------------------------------+-------------------------------------------------------------------------------------------------------------------+------------------------------------------------------------------------------------------------------------------------+
Extend the query
----------------
Query writing is an inherently iterative process. You write a simple query and then, when you run it, you discover examples that you had not previously considered, or opportunities for improvement.
Remove false positive results
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Browsing the results of our basic query shows that it could be improved. Among the results you are likely to find examples of ``if`` statements with an ``else`` branch, where a ``pass`` statement does serve a purpose. For example:
.. code-block:: python
if cond():
pass
else:
do_something()
In this case, identifying the ``if`` statement with the ``pass`` statement as redundant is a false positive. One solution to this is to modify the query to ignore ``pass`` statements if the ``if`` statement has an ``else`` branch.
To exclude ``if`` statements that have an ``else`` branch:
#. Extend the ``where`` clause to include the following extra condition:
.. code-block:: ql
and not exists(ifstmt.getOrelse())
The ``where`` clause is now:
.. code-block:: ql
where pass = ifstmt.getStmt(0) and
pass instanceof Pass and
not exists(ifstmt.getOrelse())
#. Click **Run**.
There are now fewer results because ``if`` statements with an ``else`` branch are no longer included.
`See this in the query console <https://lgtm.com/query/3424727946018612474/>`__
Further reading
---------------
.. include:: ../../reusables/python-further-reading.rst
.. include:: ../../reusables/codeql-ref-tools-further-reading.rst

View File

@@ -6,6 +6,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
.. toctree::
:hidden:
basic-query-python
introduce-libraries-python
functions
statements-expressions
@@ -13,7 +14,7 @@ Experiment and learn how to write effective and efficient queries for CodeQL dat
control-flow
taint-tracking
- `Basic Python query <https://lgtm.com/help/lgtm/console/ql-python-basic-example>`__ : Learn to write and run a simple CodeQL query using LGTM.
- :doc:`Basic query for Python code <basic-query-python>`: Learn to write and run a simple CodeQL query using LGTM.
- :doc:`CodeQL library for Python <introduce-libraries-python>`: When you need to analyze a Python program, you can make use of the large collection of classes in the CodeQL library for Python.

View File

@@ -469,6 +469,6 @@ https://github.com/github/codeql/pull/1718 for details.
### Consistency checks
The file `dataflow/internal/DataFlowImplConsistency.qll` contains a number of
consistency checks to verify that the language-specfic parts satisfy the
consistency checks to verify that the language-specific parts satisfy the
invariants that are expected by the shared implementation. Run these queries to
check for inconsistencies.

View File

@@ -0,0 +1,3 @@
name: codeql-java-examples
version: 0.0.0
libraryPathDependencies: codeql-java

View File

@@ -6,4 +6,11 @@
- exclude:
tags contain:
- ide-contextual-queries/local-definitions
- ide-contextual-queries/local-references
- ide-contextual-queries/local-references
- query: Metrics/Dependencies/ExternalDependencies.ql
- query: Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
- query: Metrics/Files/FLinesOfCode.ql
- query: Metrics/Files/FLinesOfCommentedCode.ql
- query: Metrics/Files/FLinesOfComment.ql
- query: Metrics/Files/FLinesOfDuplicatedCode.ql
- query: Metrics/Files/FNumberOfTests.ql

View File

@@ -22,9 +22,15 @@ credentials are sent in an SSL session without certificate validation. In the 'G
<references>
<li>
<a href="https://cwe.mitre.org/data/definitions/297.html">CWE-297</a>
<a href="https://issues.apache.org/jira/browse/LOG4J2-2819">Add support for specifying an SSL configuration for SmtpAppender (CVE-2020-9488)</a>
<a href="https://rules.sonarsource.com/java/tag/owasp/RSPEC-4499">SMTP SSL connection should check server identity</a>
<a href="https://cwe.mitre.org/data/definitions/297.html">CWE-297</a>
</li>
<li>
Log4j2:
<a href="https://issues.apache.org/jira/browse/LOG4J2-2819">Add support for specifying an SSL configuration for SmtpAppender (CVE-2020-9488)</a>
</li>
<li>
SonarSource rule:
<a href="https://rules.sonarsource.com/java/tag/owasp/RSPEC-4499">SMTP SSL connection should check server identity</a>
</li>
</references>
</qhelp>
</qhelp>

View File

@@ -130,6 +130,7 @@ private predicate taintPreservingQualifierToMethod(Method m) {
m.(CollectionMethod).hasName(["peek", "pop"])
or
// java.util.Queue
// covered by Stack: peek()
m.(CollectionMethod).hasName(["element", "poll"])
or
m.(CollectionMethod).hasName("remove") and m.getNumberOfParameters() = 0
@@ -254,7 +255,7 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
// covered by Deque: offerFirst(E, long, TimeUnit), offerLast(E, long, TimeUnit)
method.(CollectionMethod).hasName(["putFirst", "putLast"]) and arg = 0
or
//java.util.Dictionary
// java.util.Dictionary
method
.getDeclaringType()
.getSourceDeclaration()
@@ -269,9 +270,6 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
* `arg`th argument is tainted.
*/
private predicate taintPreservingArgumentToMethod(Method method, int arg) {
// java.util.Stack
method.(CollectionMethod).hasName("push") and arg = 0
or
method.getDeclaringType().hasQualifiedName("java.util", "Collections") and
(
method

View File

@@ -413,6 +413,18 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
m.hasName("toString") and node1.asExpr() = ma.getArgument(1)
)
)
or
exists(MethodAccess ma, Method m |
ma = node2.asExpr() and
m = ma.getMethod() and
m
.getDeclaringType()
.getSourceDeclaration()
.getASourceSupertype*()
.hasQualifiedName("java.util", "Stack") and
m.hasName("push") and
node1.asExpr() = ma.getArgument(0)
)
}
/**

View File

@@ -0,0 +1,3 @@
name: codeql-javascript-examples
version: 0.0.0
libraryPathDependencies: codeql-javascript

View File

@@ -7,3 +7,11 @@
tags contain:
- ide-contextual-queries/local-definitions
- ide-contextual-queries/local-references
- query: Comments/FCommentedOutCode.ql
- query: Metrics/Dependencies/ExternalDependencies.ql
- query: Metrics/Dependencies/ExternalDependenciesSourceLinks.ql
- query: Metrics/FLinesOfCode.ql
- query: Metrics/FLinesOfComment.ql
- query: Metrics/FLinesOfDuplicatedCode.ql
- query: Metrics/FLinesOfSimilarCode.ql
- query: Metrics/FNumberOfTests.ql

View File

@@ -109,7 +109,7 @@ class Expr extends @expr, ExprOrStmt, ExprOrType, AST::ValueNode {
/** Gets the constant string value this expression evaluates to, if any. */
cached
string getStringValue() { none() }
string getStringValue() { result = getStringValue(this) }
/** Holds if this expression is impure, that is, its evaluation could have side effects. */
predicate isImpure() { any() }
@@ -423,8 +423,6 @@ class BigIntLiteral extends @bigintliteral, Literal {
* ```
*/
class StringLiteral extends @stringliteral, Literal {
override string getStringValue() { result = getValue() }
/**
* Gets the value of this string literal parsed as a regular expression, if possible.
*
@@ -839,8 +837,6 @@ class SeqExpr extends @seqexpr, Expr {
override predicate isImpure() { getAnOperand().isImpure() }
override string getStringValue() { result = getLastOperand().getStringValue() }
override Expr getUnderlyingValue() { result = getLastOperand().getUnderlyingValue() }
}
@@ -1517,13 +1513,71 @@ class URShiftExpr extends @urshiftexpr, BinaryExpr {
*/
class AddExpr extends @addexpr, BinaryExpr {
override string getOperator() { result = "+" }
override string getStringValue() {
result = getLeftOperand().getStringValue() + getRightOperand().getStringValue() and
result.length() < 1000 * 1000
}
}
/**
* Gets the string value for the expression `e`.
* This string-value is either a constant-string, or the result from a simple string-concatenation.
*/
private string getStringValue(Expr e) {
result = getConstantString(e)
or
result = getConcatenatedString(e)
}
/**
* Gets the constant string value for the expression `e`.
*/
private string getConstantString(Expr e) {
result = getConstantString(e.getUnderlyingValue())
or
result = e.(StringLiteral).getValue()
or
exists(TemplateLiteral lit | lit = e |
// fold singletons
lit.getNumChildExpr() = 0 and
result = ""
or
e.getNumChildExpr() = 1 and
result = getConstantString(lit.getElement(0))
)
or
result = e.(TemplateElement).getValue()
}
/**
* Holds if `add` is a string-concatenation where all the transitive leafs have a constant string value.
*/
private predicate hasAllConstantLeafs(AddExpr add) {
forex(Expr leaf | leaf = getAnAddOperand*(add) and not exists(getAnAddOperand(leaf)) |
exists(getConstantString(leaf))
)
}
/**
* Gets the concatenated string for a string-concatenation `add`.
* Only has a result if `add` is not itself an operand in another string-concatenation with all constant leafs.
*/
private string getConcatenatedString(Expr add) {
result = getConcatenatedString(add.getUnderlyingValue())
or
not add = getAnAddOperand(any(AddExpr parent | hasAllConstantLeafs(parent))) and
hasAllConstantLeafs(add) and
result =
strictconcat(Expr leaf |
leaf = getAnAddOperand*(add)
|
getConstantString(leaf) order by leaf.getFirstToken().getIndex()
) and
result.length() < 1000 * 1000
}
/**
* Gets an operand from `add`.
* Is specialized to `AddExpr` such that `getAnAddOperand*(add)` can be used to get a leaf from a string-concatenation transitively.
*/
private Expr getAnAddOperand(AddExpr add) { result = add.getAnOperand().getUnderlyingValue() }
/**
* A subtraction expression.
*

View File

@@ -13,7 +13,7 @@ bindingset[path]
private int countSlashes(string path) { result = count(path.splitAt("/")) - 1 }
/**
* Gets the topmost package.json that appears in the project.
* Gets the topmost named package.json that appears in the project.
*
* There can be multiple results if the there exists multiple package.json that are equally deeply nested in the folder structure.
* Results are limited to package.json files that are at most nested 2 directories deep.
@@ -21,7 +21,8 @@ private int countSlashes(string path) { result = count(path.splitAt("/")) - 1 }
PackageJSON getTopmostPackageJSON() {
result =
min(PackageJSON j |
countSlashes(j.getFile().getRelativePath()) <= 3
countSlashes(j.getFile().getRelativePath()) <= 3 and
exists(j.getPackageName())
|
j order by countSlashes(j.getFile().getRelativePath())
)

View File

@@ -57,15 +57,6 @@ class TemplateLiteral extends Expr, @templateliteral {
int getNumElement() { result = count(getAnElement()) }
override predicate isImpure() { getAnElement().isImpure() }
override string getStringValue() {
// fold singletons
getNumChildExpr() = 0 and
result = ""
or
getNumChildExpr() = 1 and
result = getElement(0).getStringValue()
}
}
/**
@@ -103,6 +94,4 @@ class TemplateElement extends Expr, @templateelement {
string getRawValue() { literals(_, result, this) }
override predicate isImpure() { none() }
override string getStringValue() { result = getValue() }
}

View File

@@ -302,6 +302,8 @@ private module Mongoose {
MongoDB::CollectionMethodSignatures::interpretsArgumentAsQuery(name, n)
or
name = "findByIdAndUpdate" and n = 1
or
name = "where" and n = 0
}
/**

View File

@@ -32,6 +32,12 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
(method = "command" or method = "commandSync")
) and
cmdArg = 0
or
mod = "execa" and
method = "node" and
cmdArg = 0 and
optionsArg = 1 and
shell = false
|
callee = DataFlow::moduleMember(mod, method) and
sync = getSync(method)
@@ -59,6 +65,12 @@ private class SystemCommandExecutors extends SystemCommandExecution, DataFlow::I
|
this = callee.getACall()
)
or
this = DataFlow::moduleImport("foreground-child").getACall() and
cmdArg = 0 and
optionsArg = 1 and
shell = false and
sync = true
}
override DataFlow::Node getACommandArgument() { result = getArgument(cmdArg) }
@@ -123,3 +135,15 @@ private class RemoteCommandExecutor extends SystemCommandExecution, DataFlow::In
override DataFlow::Node getOptionsArg() { none() }
}
private class Opener extends SystemCommandExecution, DataFlow::InvokeNode {
Opener() { this = DataFlow::moduleImport("opener").getACall() }
override DataFlow::Node getACommandArgument() { result = getOptionArgument(1, "command") }
override predicate isShellInterpreted(DataFlow::Node arg) { none() }
override predicate isSync() { none() }
override DataFlow::Node getOptionsArg() { none() }
}

View File

@@ -145,4 +145,17 @@ module ClientSideUrlRedirect {
)
}
}
/**
* A write of an attribute which may execute JavaScript code or
* exfiltrate data to an attacker controlled site.
*/
class AttributeWriteUrlSink extends ScriptUrlSink, DataFlow::ValueNode {
AttributeWriteUrlSink() {
exists(DomPropWriteNode pw |
pw.interpretsValueAsJavaScriptUrl() and
this = DataFlow::valueNode(pw.getRhs())
)
}
}
}

View File

@@ -90,7 +90,8 @@ class DomMethodCallExpr extends MethodCallExpr {
attr = "formaction" or
attr = "href" or
attr = "src" or
attr = "xlink:href"
attr = "xlink:href" or
attr = "data"
|
getArgument(argPos - 1).getStringValue().toLowerCase() = attr
)
@@ -116,6 +117,17 @@ class DomPropWriteNode extends Assignment {
lhs.getPropertyName() = "innerHTML" or
lhs.getPropertyName() = "outerHTML"
}
/**
* Holds if the assigned value is interpreted as JavaScript via javascript: protocol.
*/
predicate interpretsValueAsJavaScriptUrl() {
lhs.getPropertyName() = "action" or
lhs.getPropertyName() = "formaction" or
lhs.getPropertyName() = "href" or
lhs.getPropertyName() = "src" or
lhs.getPropertyName() = "data"
}
}
/**

View File

@@ -1,7 +1,4 @@
getTopmostPackageJSON
| absent_main/package.json:1:1:3:1 | {\\n " ... t.js"\\n} |
| esmodules/package.json:1:1:3:1 | {\\n " ... n.js"\\n} |
| lib1/package.json:1:1:3:1 | {\\n " ... n.js"\\n} |
getAValueExportedBy
| absent_main/package.json:1:1:3:1 | {\\n " ... t.js"\\n} | absent_main/index.js:1:1:1:0 | this |
| absent_main/package.json:1:1:3:1 | {\\n " ... t.js"\\n} | absent_main/index.js:1:1:1:14 | module.exports |

View File

@@ -46,6 +46,12 @@ concatenation
| tst.js:89:3:89:14 | x |
| tst.js:89:3:89:14 | x += 'three' |
| tst.js:95:7:95:30 | x.conca ... three') |
| tst.js:104:11:104:23 | "foo" + "bar" |
| tst.js:104:11:104:31 | "foo" + ... + value |
| tst.js:105:11:105:23 | value + "foo" |
| tst.js:105:11:105:31 | value + ... + "bar" |
| tst.js:106:11:106:33 | "foo" + ... "baz") |
| tst.js:106:20:106:32 | "bar" + "baz" |
concatenationOperand
| closure.js:5:1:5:37 | build(' ... 'four') |
| closure.js:5:7:5:11 | 'one' |
@@ -127,6 +133,18 @@ concatenationOperand
| tst.js:95:7:95:7 | x |
| tst.js:95:16:95:20 | 'two' |
| tst.js:95:23:95:29 | 'three' |
| tst.js:104:11:104:15 | "foo" |
| tst.js:104:11:104:23 | "foo" + "bar" |
| tst.js:104:19:104:23 | "bar" |
| tst.js:104:27:104:31 | value |
| tst.js:105:11:105:15 | value |
| tst.js:105:11:105:23 | value + "foo" |
| tst.js:105:19:105:23 | "foo" |
| tst.js:105:27:105:31 | "bar" |
| tst.js:106:11:106:15 | "foo" |
| tst.js:106:19:106:33 | ("bar" + "baz") |
| tst.js:106:20:106:24 | "bar" |
| tst.js:106:28:106:32 | "baz" |
concatenationLeaf
| closure.js:5:7:5:11 | 'one' |
| closure.js:5:14:5:18 | 'two' |
@@ -199,6 +217,16 @@ concatenationLeaf
| tst.js:95:7:95:7 | x |
| tst.js:95:16:95:20 | 'two' |
| tst.js:95:23:95:29 | 'three' |
| tst.js:104:11:104:15 | "foo" |
| tst.js:104:19:104:23 | "bar" |
| tst.js:104:27:104:31 | value |
| tst.js:105:11:105:15 | value |
| tst.js:105:19:105:23 | "foo" |
| tst.js:105:27:105:31 | "bar" |
| tst.js:106:11:106:15 | "foo" |
| tst.js:106:19:106:33 | ("bar" + "baz") |
| tst.js:106:20:106:24 | "bar" |
| tst.js:106:28:106:32 | "baz" |
concatenationNode
| closure.js:5:1:5:37 | build(' ... 'four') |
| closure.js:5:1:5:46 | build(' ... 'five' |
@@ -318,6 +346,22 @@ concatenationNode
| tst.js:95:7:95:30 | x.conca ... three') |
| tst.js:95:16:95:20 | 'two' |
| tst.js:95:23:95:29 | 'three' |
| tst.js:104:11:104:15 | "foo" |
| tst.js:104:11:104:23 | "foo" + "bar" |
| tst.js:104:11:104:31 | "foo" + ... + value |
| tst.js:104:19:104:23 | "bar" |
| tst.js:104:27:104:31 | value |
| tst.js:105:11:105:15 | value |
| tst.js:105:11:105:23 | value + "foo" |
| tst.js:105:11:105:31 | value + ... + "bar" |
| tst.js:105:19:105:23 | "foo" |
| tst.js:105:27:105:31 | "bar" |
| tst.js:106:11:106:15 | "foo" |
| tst.js:106:11:106:33 | "foo" + ... "baz") |
| tst.js:106:19:106:33 | ("bar" + "baz") |
| tst.js:106:20:106:24 | "bar" |
| tst.js:106:20:106:32 | "bar" + "baz" |
| tst.js:106:28:106:32 | "baz" |
operand
| closure.js:5:1:5:37 | build(' ... 'four') | 0 | closure.js:5:7:5:11 | 'one' |
| closure.js:5:1:5:37 | build(' ... 'four') | 1 | closure.js:5:14:5:28 | 'two' + 'three' |
@@ -421,6 +465,18 @@ operand
| tst.js:95:7:95:30 | x.conca ... three') | 0 | tst.js:95:7:95:7 | x |
| tst.js:95:7:95:30 | x.conca ... three') | 1 | tst.js:95:16:95:20 | 'two' |
| tst.js:95:7:95:30 | x.conca ... three') | 2 | tst.js:95:23:95:29 | 'three' |
| tst.js:104:11:104:23 | "foo" + "bar" | 0 | tst.js:104:11:104:15 | "foo" |
| tst.js:104:11:104:23 | "foo" + "bar" | 1 | tst.js:104:19:104:23 | "bar" |
| tst.js:104:11:104:31 | "foo" + ... + value | 0 | tst.js:104:11:104:23 | "foo" + "bar" |
| tst.js:104:11:104:31 | "foo" + ... + value | 1 | tst.js:104:27:104:31 | value |
| tst.js:105:11:105:23 | value + "foo" | 0 | tst.js:105:11:105:15 | value |
| tst.js:105:11:105:23 | value + "foo" | 1 | tst.js:105:19:105:23 | "foo" |
| tst.js:105:11:105:31 | value + ... + "bar" | 0 | tst.js:105:11:105:23 | value + "foo" |
| tst.js:105:11:105:31 | value + ... + "bar" | 1 | tst.js:105:27:105:31 | "bar" |
| tst.js:106:11:106:33 | "foo" + ... "baz") | 0 | tst.js:106:11:106:15 | "foo" |
| tst.js:106:11:106:33 | "foo" + ... "baz") | 1 | tst.js:106:19:106:33 | ("bar" + "baz") |
| tst.js:106:20:106:32 | "bar" + "baz" | 0 | tst.js:106:20:106:24 | "bar" |
| tst.js:106:20:106:32 | "bar" + "baz" | 1 | tst.js:106:28:106:32 | "baz" |
nextLeaf
| closure.js:5:7:5:11 | 'one' | closure.js:5:14:5:18 | 'two' |
| closure.js:5:14:5:18 | 'two' | closure.js:5:22:5:28 | 'three' |
@@ -466,6 +522,12 @@ nextLeaf
| tst.js:89:3:89:3 | x | tst.js:89:8:89:14 | 'three' |
| tst.js:95:7:95:7 | x | tst.js:95:16:95:20 | 'two' |
| tst.js:95:16:95:20 | 'two' | tst.js:95:23:95:29 | 'three' |
| tst.js:104:11:104:15 | "foo" | tst.js:104:19:104:23 | "bar" |
| tst.js:104:19:104:23 | "bar" | tst.js:104:27:104:31 | value |
| tst.js:105:11:105:15 | value | tst.js:105:19:105:23 | "foo" |
| tst.js:105:19:105:23 | "foo" | tst.js:105:27:105:31 | "bar" |
| tst.js:106:11:106:15 | "foo" | tst.js:106:19:106:33 | ("bar" + "baz") |
| tst.js:106:20:106:24 | "bar" | tst.js:106:28:106:32 | "baz" |
htmlRoot
| html-concat.js:2:14:2:26 | `<b>${x}</b>` |
| html-concat.js:3:14:3:26 | `<B>${x}</B>` |
@@ -488,3 +550,13 @@ htmlLeaf
| html-concat.js:8:15:10:23 | .\\n \\n ... um!</i> |
| html-concat.js:13:3:13:8 | buffer |
| html-concat.js:13:13:13:18 | '<li>' |
getStringValue
| tst.js:104:11:104:15 | "foo" | foo |
| tst.js:104:11:104:23 | "foo" + "bar" | foobar |
| tst.js:104:19:104:23 | "bar" | bar |
| tst.js:105:19:105:23 | "foo" | foo |
| tst.js:105:27:105:31 | "bar" | bar |
| tst.js:106:11:106:15 | "foo" | foo |
| tst.js:106:11:106:33 | "foo" + ... "baz") | foobarbaz |
| tst.js:106:20:106:24 | "bar" | bar |
| tst.js:106:28:106:32 | "baz" | baz |

View File

@@ -19,3 +19,8 @@ query predicate nextLeaf(StringOps::ConcatenationNode node, DataFlow::Node next)
query StringOps::HtmlConcatenationRoot htmlRoot() { any() }
query StringOps::HtmlConcatenationLeaf htmlLeaf() { any() }
query string getStringValue(Expr e) {
result = e.getStringValue() and
e.getEnclosingFunction().getName() = "stringValue"
}

View File

@@ -99,3 +99,9 @@ function concatCall() {
function arrayConcat(a, b) {
return [].concat(a, b);
}
function stringValue() {
var a = "foo" + "bar" + value;
var b = value + "foo" + "bar";
var c = "foo" + ("bar" + "baz")
}

View File

@@ -29,30 +29,32 @@ nodes
| child_process-test.js:39:26:39:28 | cmd |
| child_process-test.js:43:15:43:17 | cmd |
| child_process-test.js:43:15:43:17 | cmd |
| child_process-test.js:50:15:50:17 | cmd |
| child_process-test.js:50:15:50:17 | cmd |
| child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) |
| child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) |
| child_process-test.js:53:46:53:57 | ["bar", cmd] |
| child_process-test.js:53:46:53:57 | ["bar", cmd] |
| child_process-test.js:53:54:53:56 | cmd |
| child_process-test.js:53:54:53:56 | cmd |
| child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
| child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
| child_process-test.js:54:46:54:48 | cmd |
| child_process-test.js:70:9:70:49 | cmd |
| child_process-test.js:70:15:70:38 | url.par ... , true) |
| child_process-test.js:70:15:70:44 | url.par ... ).query |
| child_process-test.js:70:15:70:49 | url.par ... ry.path |
| child_process-test.js:70:25:70:31 | req.url |
| child_process-test.js:70:25:70:31 | req.url |
| child_process-test.js:72:29:72:31 | cmd |
| child_process-test.js:72:29:72:31 | cmd |
| child_process-test.js:80:19:80:36 | req.query.fileName |
| child_process-test.js:80:19:80:36 | req.query.fileName |
| child_process-test.js:80:19:80:36 | req.query.fileName |
| child_process-test.js:82:37:82:54 | req.query.fileName |
| child_process-test.js:82:37:82:54 | req.query.fileName |
| child_process-test.js:48:15:48:17 | cmd |
| child_process-test.js:48:15:48:17 | cmd |
| child_process-test.js:53:15:53:17 | cmd |
| child_process-test.js:53:15:53:17 | cmd |
| child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) |
| child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) |
| child_process-test.js:56:46:56:57 | ["bar", cmd] |
| child_process-test.js:56:46:56:57 | ["bar", cmd] |
| child_process-test.js:56:54:56:56 | cmd |
| child_process-test.js:56:54:56:56 | cmd |
| child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) |
| child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) |
| child_process-test.js:57:46:57:48 | cmd |
| child_process-test.js:73:9:73:49 | cmd |
| child_process-test.js:73:15:73:38 | url.par ... , true) |
| child_process-test.js:73:15:73:44 | url.par ... ).query |
| child_process-test.js:73:15:73:49 | url.par ... ry.path |
| child_process-test.js:73:25:73:31 | req.url |
| child_process-test.js:73:25:73:31 | req.url |
| child_process-test.js:75:29:75:31 | cmd |
| child_process-test.js:75:29:75:31 | cmd |
| child_process-test.js:83:19:83:36 | req.query.fileName |
| child_process-test.js:83:19:83:36 | req.query.fileName |
| child_process-test.js:83:19:83:36 | req.query.fileName |
| child_process-test.js:85:37:85:54 | req.query.fileName |
| child_process-test.js:85:37:85:54 | req.query.fileName |
| execSeries.js:3:20:3:22 | arr |
| execSeries.js:6:14:6:16 | arr |
| execSeries.js:6:14:6:21 | arr[i++] |
@@ -109,6 +111,12 @@ nodes
| other.js:23:28:23:30 | cmd |
| other.js:26:34:26:36 | cmd |
| other.js:26:34:26:36 | cmd |
| other.js:28:27:28:29 | cmd |
| other.js:28:27:28:29 | cmd |
| other.js:30:33:30:35 | cmd |
| other.js:30:33:30:35 | cmd |
| other.js:34:44:34:46 | cmd |
| other.js:34:44:34:46 | cmd |
| third-party-command-injection.js:5:20:5:26 | command |
| third-party-command-injection.js:5:20:5:26 | command |
| third-party-command-injection.js:6:21:6:27 | command |
@@ -133,11 +141,13 @@ edges
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:39:26:39:28 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:43:15:43:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:43:15:43:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:50:15:50:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:50:15:50:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:53:54:53:56 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:53:54:53:56 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:54:46:54:48 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:48:15:48:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:48:15:48:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:53:15:53:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:53:15:53:17 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:56:54:56:56 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:56:54:56:56 | cmd |
| child_process-test.js:6:9:6:49 | cmd | child_process-test.js:57:46:57:48 | cmd |
| child_process-test.js:6:15:6:38 | url.par ... , true) | child_process-test.js:6:15:6:44 | url.par ... ).query |
| child_process-test.js:6:15:6:44 | url.par ... ).query | child_process-test.js:6:15:6:49 | url.par ... ry.path |
| child_process-test.js:6:15:6:44 | url.par ... ).query | child_process-test.js:6:15:6:49 | url.par ... ry.path |
@@ -148,22 +158,22 @@ edges
| child_process-test.js:25:21:25:23 | cmd | child_process-test.js:25:13:25:31 | "foo" + cmd + "bar" |
| child_process-test.js:39:26:39:28 | cmd | child_process-test.js:39:18:39:30 | [ flag, cmd ] |
| child_process-test.js:39:26:39:28 | cmd | child_process-test.js:39:18:39:30 | [ flag, cmd ] |
| child_process-test.js:53:46:53:57 | ["bar", cmd] | child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) |
| child_process-test.js:53:46:53:57 | ["bar", cmd] | child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) |
| child_process-test.js:53:54:53:56 | cmd | child_process-test.js:53:46:53:57 | ["bar", cmd] |
| child_process-test.js:53:54:53:56 | cmd | child_process-test.js:53:46:53:57 | ["bar", cmd] |
| child_process-test.js:54:46:54:48 | cmd | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
| child_process-test.js:54:46:54:48 | cmd | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
| child_process-test.js:70:9:70:49 | cmd | child_process-test.js:72:29:72:31 | cmd |
| child_process-test.js:70:9:70:49 | cmd | child_process-test.js:72:29:72:31 | cmd |
| child_process-test.js:70:15:70:38 | url.par ... , true) | child_process-test.js:70:15:70:44 | url.par ... ).query |
| child_process-test.js:70:15:70:44 | url.par ... ).query | child_process-test.js:70:15:70:49 | url.par ... ry.path |
| child_process-test.js:70:15:70:49 | url.par ... ry.path | child_process-test.js:70:9:70:49 | cmd |
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
| child_process-test.js:70:25:70:31 | req.url | child_process-test.js:70:15:70:38 | url.par ... , true) |
| child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName |
| child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
| child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
| child_process-test.js:56:46:56:57 | ["bar", cmd] | child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) |
| child_process-test.js:56:46:56:57 | ["bar", cmd] | child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) |
| child_process-test.js:56:54:56:56 | cmd | child_process-test.js:56:46:56:57 | ["bar", cmd] |
| child_process-test.js:56:54:56:56 | cmd | child_process-test.js:56:46:56:57 | ["bar", cmd] |
| child_process-test.js:57:46:57:48 | cmd | child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) |
| child_process-test.js:57:46:57:48 | cmd | child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) |
| child_process-test.js:73:9:73:49 | cmd | child_process-test.js:75:29:75:31 | cmd |
| child_process-test.js:73:9:73:49 | cmd | child_process-test.js:75:29:75:31 | cmd |
| child_process-test.js:73:15:73:38 | url.par ... , true) | child_process-test.js:73:15:73:44 | url.par ... ).query |
| child_process-test.js:73:15:73:44 | url.par ... ).query | child_process-test.js:73:15:73:49 | url.par ... ry.path |
| child_process-test.js:73:15:73:49 | url.par ... ry.path | child_process-test.js:73:9:73:49 | cmd |
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) |
| child_process-test.js:73:25:73:31 | req.url | child_process-test.js:73:15:73:38 | url.par ... , true) |
| child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName |
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
| child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:7:32:7:35 | name |
| execSeries.js:3:20:3:22 | arr | execSeries.js:6:14:6:16 | arr |
| execSeries.js:6:14:6:16 | arr | execSeries.js:6:14:6:21 | arr[i++] |
| execSeries.js:6:14:6:21 | arr[i++] | execSeries.js:14:24:14:30 | command |
@@ -213,6 +223,12 @@ edges
| other.js:5:9:5:49 | cmd | other.js:23:28:23:30 | cmd |
| other.js:5:9:5:49 | cmd | other.js:26:34:26:36 | cmd |
| other.js:5:9:5:49 | cmd | other.js:26:34:26:36 | cmd |
| other.js:5:9:5:49 | cmd | other.js:28:27:28:29 | cmd |
| other.js:5:9:5:49 | cmd | other.js:28:27:28:29 | cmd |
| other.js:5:9:5:49 | cmd | other.js:30:33:30:35 | cmd |
| other.js:5:9:5:49 | cmd | other.js:30:33:30:35 | cmd |
| other.js:5:9:5:49 | cmd | other.js:34:44:34:46 | cmd |
| other.js:5:9:5:49 | cmd | other.js:34:44:34:46 | cmd |
| other.js:5:15:5:38 | url.par ... , true) | other.js:5:15:5:44 | url.par ... ).query |
| other.js:5:15:5:44 | url.par ... ).query | other.js:5:15:5:49 | url.par ... ry.path |
| other.js:5:15:5:49 | url.par ... ry.path | other.js:5:9:5:49 | cmd |
@@ -234,18 +250,18 @@ edges
| child_process-test.js:39:5:39:31 | cp.spaw ... cmd ]) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:39:18:39:30 | [ flag, cmd ] | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:39:5:39:31 | cp.spaw ... cmd ]) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:39:26:39:28 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:44:5:44:34 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:51:5:51:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:50:15:50:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:53:5:53:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:53:5:53:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:53:46:53:57 | ["bar", cmd] | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:53:5:53:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:53:54:53:56 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:54:5:54:50 | cp.spaw ... t(cmd)) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:6:15:6:49 | url.par ... ry.path | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:54:5:54:50 | cp.spaw ... t(cmd)) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:59:5:59:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:50:15:50:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:64:3:64:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:43:15:43:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:72:29:72:31 | cmd | child_process-test.js:70:25:70:31 | req.url | child_process-test.js:72:29:72:31 | cmd | This command depends on $@. | child_process-test.js:70:25:70:31 | req.url | a user-provided value |
| child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName | child_process-test.js:80:19:80:36 | req.query.fileName | This command depends on $@. | child_process-test.js:80:19:80:36 | req.query.fileName | a user-provided value |
| child_process-test.js:54:5:54:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:53:15:53:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:56:5:56:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:56:5:56:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:56:46:56:57 | ["bar", cmd] | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:56:5:56:59 | cp.spaw ... cmd])) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:56:54:56:56 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:57:5:57:50 | cp.spaw ... t(cmd)) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:6:15:6:49 | url.par ... ry.path | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:57:5:57:50 | cp.spaw ... t(cmd)) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:62:5:62:39 | cp.exec ... , args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:53:15:53:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:67:3:67:21 | cp.spawn(cmd, args) | child_process-test.js:6:25:6:31 | req.url | child_process-test.js:48:15:48:17 | cmd | This command depends on $@. | child_process-test.js:6:25:6:31 | req.url | a user-provided value |
| child_process-test.js:75:29:75:31 | cmd | child_process-test.js:73:25:73:31 | req.url | child_process-test.js:75:29:75:31 | cmd | This command depends on $@. | child_process-test.js:73:25:73:31 | req.url | a user-provided value |
| child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName | child_process-test.js:83:19:83:36 | req.query.fileName | This command depends on $@. | child_process-test.js:83:19:83:36 | req.query.fileName | a user-provided value |
| execSeries.js:14:41:14:47 | command | execSeries.js:18:34:18:40 | req.url | execSeries.js:14:41:14:47 | command | This command depends on $@. | execSeries.js:18:34:18:40 | req.url | a user-provided value |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | child_process-test.js:82:37:82:54 | req.query.fileName | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:82:37:82:54 | req.query.fileName | a user-provided value |
| lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | child_process-test.js:85:37:85:54 | req.query.fileName | lib/subLib/index.js:8:10:8:25 | "rm -rf " + name | This command depends on $@. | child_process-test.js:85:37:85:54 | req.query.fileName | a user-provided value |
| other.js:7:33:7:35 | cmd | other.js:5:25:5:31 | req.url | other.js:7:33:7:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:8:28:8:30 | cmd | other.js:5:25:5:31 | req.url | other.js:8:28:8:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:9:32:9:34 | cmd | other.js:5:25:5:31 | req.url | other.js:9:32:9:34 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
@@ -261,4 +277,7 @@ edges
| other.js:22:21:22:23 | cmd | other.js:5:25:5:31 | req.url | other.js:22:21:22:23 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:23:28:23:30 | cmd | other.js:5:25:5:31 | req.url | other.js:23:28:23:30 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:26:34:26:36 | cmd | other.js:5:25:5:31 | req.url | other.js:26:34:26:36 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:28:27:28:29 | cmd | other.js:5:25:5:31 | req.url | other.js:28:27:28:29 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:30:33:30:35 | cmd | other.js:5:25:5:31 | req.url | other.js:30:33:30:35 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| other.js:34:44:34:46 | cmd | other.js:5:25:5:31 | req.url | other.js:34:44:34:46 | cmd | This command depends on $@. | other.js:5:25:5:31 | req.url | a user-provided value |
| third-party-command-injection.js:6:21:6:27 | command | third-party-command-injection.js:5:20:5:26 | command | third-party-command-injection.js:6:21:6:27 | command | This command depends on $@. | third-party-command-injection.js:5:20:5:26 | command | a server-provided value |

View File

@@ -0,0 +1,22 @@
import javascript
import testUtilities.ConsistencyChecking
import semmle.javascript.security.dataflow.CommandInjection
import semmle.javascript.security.dataflow.IndirectCommandInjection
import semmle.javascript.security.dataflow.ShellCommandInjectionFromEnvironment
import semmle.javascript.security.dataflow.UnsafeShellCommandConstruction
class CommandInjectionConsistency extends ConsistencyConfiguration {
CommandInjectionConsistency() { this = "ComandInjection" }
override File getAFile() { not result.getBaseName() = "uselesscat.js" }
}
import semmle.javascript.security.UselessUseOfCat
class UselessCatConsistency extends ConsistencyConfiguration {
UselessCatConsistency() { this = "Cat" }
override DataFlow::Node getAnAlert() { result instanceof UselessCat }
override File getAFile() { result.getBaseName() = "uselesscat.js" }
}

View File

@@ -56,6 +56,7 @@ syncCommand
| other.js:7:5:7:36 | require ... nc(cmd) |
| other.js:9:5:9:35 | require ... nc(cmd) |
| other.js:12:5:12:30 | require ... nc(cmd) |
| other.js:30:5:30:36 | require ... ")(cmd) |
| third-party-command-injection.js:6:9:6:28 | cp.execSync(command) |
| tst_shell-command-injection-from-environment.js:4:2:4:62 | cp.exec ... emp")]) |
| tst_shell-command-injection-from-environment.js:5:2:5:54 | cp.exec ... temp")) |
@@ -90,9 +91,9 @@ syncCommand
| uselesscat.js:158:16:158:46 | cspawn. ... /bar']) |
| uselesscat.js:159:16:159:68 | cspawn. ... tf8' }) |
options
| child_process-test.js:53:5:53:59 | cp.spaw ... cmd])) | child_process-test.js:53:25:53:58 | ['/C', ... , cmd]) |
| child_process-test.js:54:5:54:50 | cp.spaw ... t(cmd)) | child_process-test.js:54:25:54:49 | ['/C', ... at(cmd) |
| child_process-test.js:64:3:64:21 | cp.spawn(cmd, args) | child_process-test.js:64:17:64:20 | args |
| child_process-test.js:56:5:56:59 | cp.spaw ... cmd])) | child_process-test.js:56:25:56:58 | ['/C', ... , cmd]) |
| child_process-test.js:57:5:57:50 | cp.spaw ... t(cmd)) | child_process-test.js:57:25:57:49 | ['/C', ... at(cmd) |
| child_process-test.js:67:3:67:21 | cp.spawn(cmd, args) | child_process-test.js:67:17:67:20 | args |
| lib/lib.js:152:2:152:23 | cp.spaw ... gs, cb) | lib/lib.js:152:21:152:22 | cb |
| lib/lib.js:159:2:159:23 | cp.spaw ... gs, cb) | lib/lib.js:159:21:159:22 | cb |
| lib/lib.js:163:2:167:2 | cp.spaw ... t' }\\n\\t) | lib/lib.js:166:3:166:22 | { stdio: 'inherit' } |
@@ -114,6 +115,3 @@ options
| uselesscat.js:156:1:156:35 | cspawn( ... tf8' }) | uselesscat.js:156:15:156:34 | { encoding: 'utf8' } |
| uselesscat.js:159:16:159:68 | cspawn. ... tf8' }) | uselesscat.js:159:48:159:67 | { encoding: 'utf8' } |
| uselesscat.js:164:1:164:76 | execmod ... (out)}) | uselesscat.js:164:24:164:41 | {encoding: 'utf8'} |
#select
| False negative | uselesscat.js:54:42:54:69 | // NOT ... lagged] |
| False positive | uselesscat.js:44:37:44:85 | // OK [ ... le read |

View File

@@ -1,22 +1,6 @@
import javascript
import semmle.javascript.security.UselessUseOfCat
from LineComment comment, string msg
where
comment.getFile().getAbsolutePath().regexpMatch(".*/uselesscat.js") and
(
comment.getText().regexpMatch(".*NOT OK.*") and
not any(UselessCat cat).asExpr().getLocation().getStartLine() =
comment.getLocation().getStartLine() and
msg = "False negative"
or
comment.getText().regexpMatch(".* OK.*") and
not comment.getText().regexpMatch(".*NOT OK.*") and
any(UselessCat cat).asExpr().getLocation().getStartLine() = comment.getLocation().getStartLine() and
msg = "False positive"
)
select msg, comment
query string readFile(UselessCat cat) { result = PrettyPrintCatCall::createReadFileCall(cat) }
query SystemCommandExecution syncCommand() { result.isSync() }

View File

@@ -40,15 +40,18 @@ var server = http.createServer(function(req, res) {
let args = [];
args[0] = "-c";
args[1] = cmd;
cp.execFile("/bin/bash", args); // NOT OK
args[1] = cmd; // NOT OK
cp.execFile("/bin/bash", args);
let args = [];
args[0] = "-c";
args[1] = cmd; // NOT OK
run("sh", args);
let args = [];
args[0] = `-` + "c";
args[1] = cmd;
cp.execFile(`/bin` + "/bash", args); // NOT OK
args[1] = cmd; // NOT OK
cp.execFile(`/bin` + "/bash", args);
cp.spawn('cmd.exe', ['/C', 'foo'].concat(["bar", cmd])); // NOT OK
cp.spawn('cmd.exe', ['/C', 'foo'].concat(cmd)); // NOT OK
@@ -56,12 +59,12 @@ var server = http.createServer(function(req, res) {
let myArgs = [];
myArgs.push(`-` + "c");
myArgs.push(cmd);
cp.execFile(`/bin` + "/bash", args); // NOT OK
cp.execFile(`/bin` + "/bash", args); // NOT OK - but no support for `[].push()` for indirect arguments [INCONSISTENCY]
});
function run(cmd, args) {
cp.spawn(cmd, args); // NOT OK
cp.spawn(cmd, args); // OK - the alert happens where `args` is build.
}
var util = require("util")

View File

@@ -16,7 +16,7 @@ var cp = require("child_process");
cp.execSync("cmd.sh " + fewerArgs[0]); // NOT OK
var arg0 = fewerArgs[0];
cp.execSync(arg0); // OK
cp.execSync(arg0); // NOT OK
cp.execSync("cmd.sh " + arg0); // NOT OK
});

View File

@@ -11,10 +11,10 @@ function asyncEach(arr, iterator) {
}
function execEach(commands) {
asyncEach(commands, (command) => exec(command));
asyncEach(commands, (command) => exec(command)); // NOT OK
};
require('http').createServer(function(req, res) {
let cmd = require('url').parse(req.url, true).query.path;
execEach([cmd]); // NOT OK
execEach([cmd]);
});

Some files were not shown because too many files have changed in this diff Show More