From 3de1f3b101b91edd2b5edd89fa6b37b1ae95bfa0 Mon Sep 17 00:00:00 2001 From: calum Date: Mon, 15 Oct 2018 12:20:10 +0100 Subject: [PATCH 01/22] C#: Query and qltest for VulnerablePackage. --- .../CWE-937/Vulnerabilities.qll | 291 ++++++++++++++++++ .../CWE-937/Vulnerability.qll | 94 ++++++ .../CWE-937/VulnerablePackage.ql | 19 ++ .../Security Features/CWE-937/Program.cs | 0 .../CWE-937/VulnerablePackage.expected | 6 + .../CWE-937/VulnerablePackage.qlref | 1 + .../Security Features/CWE-937/csproj.config | 16 + .../Security Features/CWE-937/packages.config | 10 + 8 files changed, 437 insertions(+) create mode 100644 csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll create mode 100644 csharp/ql/src/Security Features/CWE-937/Vulnerability.qll create mode 100644 csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-937/Program.cs create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.qlref create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config create mode 100644 csharp/ql/test/query-tests/Security Features/CWE-937/packages.config diff --git a/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll new file mode 100644 index 000000000000..39b94e599088 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll @@ -0,0 +1,291 @@ +/** + * A list of NuGet packages with known vulnerabilities. + * + * To add a new vulnerability follow the existing pattern. + * Create a new class that extends the abstract class `Vulnerability`, + * supplying the name and the URL, and override one (or both) of + * `matchesRange` and `matchesVersion`. + */ + +import csharp +import Vulnerability + +class MicrosoftAdvisory4021279 extends Vulnerability { + + MicrosoftAdvisory4021279() { this = "Microsoft Security Advisory 4021279" } + + override string getUrl() { result = "https://github.com/dotnet/corefx/issues/19535" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + name = "System.Text.Encodings.Web" and ( + affected = "4.0.0" and fixed = "4.0.1" + or + affected = "4.3.0" and fixed = "4.3.1" + ) or + name = "System.Net.Http" and ( + affected = "4.1.1" and fixed = "4.1.2" + or + affected = "4.3.1" and fixed = "4.3.2" + ) or + name = "System.Net.Http.WinHttpHandler" and ( + affected = "4.0.1" and fixed = "4.0.2" + or + affected = "4.3.0" and fixed = "4.3.1" + ) or + name = "System.Net.Security" and ( + affected = "4.0.0" and fixed = "4.0.1" + or + affected = "4.3.0" and fixed = "4.3.1" + ) or ( + name = "Microsoft.AspNetCore.Mvc" + or + name = "Microsoft.AspNetCore.Mvc.Core" + or + name = "Microsoft.AspNetCore.Mvc.Abstractions" + or + name = "Microsoft.AspNetCore.Mvc.ApiExplorer" + or + name = "Microsoft.AspNetCore.Mvc.Cors" + or + name = "Microsoft.AspNetCore.Mvc.DataAnnotations" + or + name = "Microsoft.AspNetCore.Mvc.Formatters.Json" + or + name = "Microsoft.AspNetCore.Mvc.Formatters.Xml" + or + name = "Microsoft.AspNetCore.Mvc.Localization" + or + name = "Microsoft.AspNetCore.Mvc.Razor.Host" + or + name = "Microsoft.AspNetCore.Mvc.Razor" + or + name = "Microsoft.AspNetCore.Mvc.TagHelpers" + or + name = "Microsoft.AspNetCore.Mvc.ViewFeatures" + or + name = "Microsoft.AspNetCore.Mvc.WebApiCompatShim" + ) and ( + affected = "1.0.0" and fixed = "1.0.4" + or + affected = "1.1.0" and fixed = "1.1.3" + ) + } +} + +class CVE_2017_8700 extends Vulnerability { + CVE_2017_8700() { this = "CVE-2017-8700" } + + override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/279" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + ( + name = "Microsoft.AspNetCore.Mvc.Core" + or + name = "Microsoft.AspNetCore.Mvc.Cors" + ) and ( + affected = "1.0.0" and fixed = "1.0.6" + or + affected = "1.1.0" and fixed = "1.1.6" + ) + } +} + +class CVE_2018_0765 extends Vulnerability { + CVE_2018_0765() { this = "CVE-2018-0765" } + + override string getUrl() { result = "https://github.com/dotnet/announcements/issues/67" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + name = "System.Security.Cryptography.Xml" and + affected = "0.0.0" and + fixed = "4.4.2" + } +} + +class AspNetCore_Mar18 extends Vulnerability { + AspNetCore_Mar18() { this = "ASPNETCore-Mar18" } + + override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/300" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + ( + name = "Microsoft.AspNetCore.Server.Kestrel.Core" + or + name = "Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions" + or + name = "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" + or + name = "Microsoft.AspNetCore.All" + ) and + affected = "2.0.0" and + fixed = "2.0.3" + } +} + +class CVE_2018_8409 extends Vulnerability { + CVE_2018_8409() { this = "CVE-2018-8409" } + + override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/316" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + name = "System.IO.Pipelines" and affected = "4.5.0" and fixed = "4.5.1" + or + (name = "Microsoft.AspNetCore.All" or name = "Microsoft.AspNetCore.App") and + affected = "2.1.0" and fixed = "2.1.4" + } +} + +class CVE_2018_8171 extends Vulnerability { + CVE_2018_8171() { this = "CVE-2018-8171" } + + override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/310" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + name = "Microsoft.AspNetCore.Identity" and ( + affected = "1.0.0" and fixed = "1.0.6" + or + affected = "1.1.0" and fixed = "1.1.6" + or + affected = "2.0.0" and fixed = "2.0.4" + or + affected = "2.1.0" and fixed = "2.1.2" + ) + } +} + +class CVE_2018_8356 extends Vulnerability { + CVE_2018_8356() { this = "CVE-2018-8356" } + + override string getUrl() { result = "https://github.com/dotnet/announcements/issues/73" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + ( + name = "System.Private.ServiceModel" + or + name = "System.ServiceModel.Http" + or + name = "System.ServiceModel.NetTcp" + ) and ( + affected = "4.0.0" and fixed = "4.1.3" + or + affected = "4.3.0" and fixed = "4.3.3" + or + affected = "4.4.0" and fixed = "4.4.4" + or + affected = "4.5.0" and fixed = "4.5.3" + ) + or + ( + name = "System.ServiceModel.Duplex" + or + name = "System.ServiceModel.Security" + ) and ( + affected = "4.0.0" and fixed = "4.0.4" + or + affected = "4.3.0" and fixed = "4.3.3" + or + affected = "4.4.0" and fixed = "4.4.4" + or + affected = "4.5.0" and fixed = "4.5.3" + ) + or + name = "System.ServiceModel.NetTcp" and ( + affected = "4.0.0" and fixed = "4.1.3" + or + affected = "4.3.0" and fixed = "4.3.3" + or + affected = "4.4.0" and fixed = "4.4.4" + or + affected = "4.5.0" and fixed = "4.5.1" + ) + } +} + +class ASPNETCore_Jul18 extends Vulnerability { + ASPNETCore_Jul18() { this = "ASPNETCore-July18" } + + override string getUrl() { result = "https://github.com/aspnet/Announcements/issues/311" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + name = "Microsoft.AspNetCore.Server.Kestrel.Core" and ( + affected = "2.0.0" and fixed = "2.0.4" + or + affected = "2.1.0" and fixed = "2.1.2" + ) + or + name = "Microsoft.AspNetCore.All" and ( + affected = "2.0.0" and fixed = "2.0.9" + or + affected = "2.1.0" and fixed = "2.1.2" + ) + or + name = "Microsoft.AspNetCore.App" and + affected = "2.1.0" and + fixed = "2.1.2" + } +} + +class CVE_2018_8292 extends Vulnerability { + CVE_2018_8292() { this = "CVE-2018-8292" } + + override string getUrl() { result = "https://github.com/dotnet/announcements/issues/88" } + + override predicate matchesVersion(string name, Version affected, Version fixed) { + name = "System.Net.Http" and ( + affected = "2.0" or + affected = "4.0.0" or + affected = "4.1.0" or + affected = "1.1.1" or + affected = "4.1.2" or + affected = "4.3.0" or + affected = "4.3.1" or + affected = "4.3.2" or + affected = "4.3.3" + ) and + fixed = "4.3.4" + } +} + +class CVE_2018_0786 extends Vulnerability { + CVE_2018_0786() { this = "CVE-2018-0786" } + + override string getUrl() { result = "https://github.com/dotnet/announcements/issues/51" } + + override predicate matchesRange(string name, Version affected, Version fixed) { + ( + name = "System.ServiceModel.Primitives" + or + name = "System.ServiceModel.Http" + or + name = "System.ServiceModel.NetTcp" + or + name = "System.ServiceModel.Duplex" + or + name = "System.ServiceModel.Security" + or + name = "System.Private.ServiceModel" + ) and ( + affected = "4.4.0" and fixed = "4.4.1" + or + affected = "4.3.0" and fixed = "4.3.1" + ) + or ( + name = "System.ServiceModel.Primitives" + or + name = "System.ServiceModel.Http" + or + name = "System.ServiceModel.NetTcp" + or + name = "System.Private.ServiceModel" + ) and + affected = "4.1.0" and + fixed = "4.1.1" + or ( + name = "System.ServiceModel.Duplex" + or + name = "System.ServiceModel.Security" + ) and + affected = "4.0.1" and + fixed = "4.0.2" + } +} diff --git a/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll b/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll new file mode 100644 index 000000000000..4a58a95efe97 --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll @@ -0,0 +1,94 @@ +import csharp + +/** + * A package reference in an XML file, for example in a + * .csproj file, a .props file or a packages.config file. + */ +class Package extends XMLElement { + string name; + Version version; + + Package() { + (this.getName() = "PackageManagement" or this.getName() = "PackageReference") and + name = this.getAttributeValue("Include") and + version = this.getAttributeValue("Version") + or + this.getName() = "package" and + name = this.getAttributeValue("id") and + version = this.getAttributeValue("version") + } + + /** Gets the name of the package, for example `System.IO.Pipelines`. */ + string getPackageName() { + result = name + } + + /** Gets the version of the package, for example `4.5.1`. */ + Version getVersion() { + result = version + } + + override string toString() { + result = name + " " + version + } +} + +/** + * A vulnerability, where the name of the vulnerability is this string. + * One of `matchesRange` or `matchesVersion` must be overridden in order to + * specify which packages are vulnerable. + */ +abstract class Vulnerability extends string { + bindingset[this] + Vulnerability() { any() } + + /** + * A package with name `name` is vulnerable from version `affected` + * until version `fixed`. + */ + predicate matchesRange(string name, Version affected, Version fixed) { none() } + + /** + * A package with name `name` is vulnerable in version `affected`, and + * is fixed by version `fixed`. + */ + predicate matchesVersion(string name, Version affecter, Version fixed) { none() } + + /** Gets the URL describing the vulnerability. */ + abstract string getUrl(); + + /** + * Holds if a package with name `name` and version `version` + * has this vulnerability. The fixed version is given by `fixed`. + */ + bindingset[name, version] + predicate isVulnerable(string name, Version version, Version fixed) { + exists(Version affected, string n | + name.toLowerCase() = n.toLowerCase() | + matchesRange(n, affected, fixed) and + version.compareTo(fixed) < 0 and + version.compareTo(affected) >= 0 + or + matchesVersion(n, affected, fixed) and + version.compareTo(affected) = 0 + ) + } +} + +/** + * A package with a vulnerability. + */ +class VulnerablePackage extends Package { + Vulnerability vuln; + Version fixed; + + VulnerablePackage() { + vuln.isVulnerable(this.getPackageName(), this.getVersion(), fixed) + } + + /** Gets the vulnerability of this package. */ + Vulnerability getVulnerability() { result = vuln } + + /** Gets the version of this package where the vulnerability is fixed. */ + Version getFixedVersion() { result = fixed } +} diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql new file mode 100644 index 000000000000..a1803d488a9e --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql @@ -0,0 +1,19 @@ +/** + * @name Using a package with a known vulnerability. + * @description Using a package with a known vulnerability is a security risk. + * Upgrade the package to a version that does not contain the vulnerability. + * @kind problem + * @problem.severity error + * @precision high + * @id cs/use-of-vulnerable-package + * @tags security + * external/cwe/cwe-937 + */ + +import csharp +import Vulnerabilities + +from Vulnerability vuln, VulnerablePackage package +where vuln = package.getVulnerability() +select package, "Package " + package + " has vulnerability $@, and should be upgraded to version " + package.getFixedVersion() + ".", + vuln.getUrl(), vuln.toString() diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/Program.cs b/csharp/ql/test/query-tests/Security Features/CWE-937/Program.cs new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected new file mode 100644 index 000000000000..b088930b7eb2 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected @@ -0,0 +1,6 @@ +| csproj.config:10:5:10:77 | System.Text.Encodings.Web 4.3.0 | Package System.Text.Encodings.Web 4.3.0 has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:11:5:11:75 | system.text.encodings.web 4.3 | Package system.text.encodings.web 4.3 has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:12:5:12:67 | System.Net.Http 4.1.1 | Package System.Net.Http 4.1.1 has vulnerability $@, and should be upgraded to version 4.1.2. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:13:5:13:67 | System.Net.Http 4.1.2 | Package System.Net.Http 4.1.2 has vulnerability $@, and should be upgraded to version 4.3.4. | https://github.com/dotnet/announcements/issues/88 | CVE-2018-8292 | +| packages.config:8:3:8:79 | System.IO.Pipelines 4.5.0 | Package System.IO.Pipelines 4.5.0 has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | +| packages.config:9:3:9:81 | System.IO.Pipelines 4.5.0.0 | Package System.IO.Pipelines 4.5.0.0 has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.qlref b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.qlref new file mode 100644 index 000000000000..fb8a73b25f95 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.qlref @@ -0,0 +1 @@ +Security Features/CWE-937/VulnerablePackage.ql \ No newline at end of file diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config b/csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config new file mode 100644 index 000000000000..721d21559407 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config @@ -0,0 +1,16 @@ + + + + + + + + + + + + + + + + diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/packages.config b/csharp/ql/test/query-tests/Security Features/CWE-937/packages.config new file mode 100644 index 000000000000..06e5ea78fd00 --- /dev/null +++ b/csharp/ql/test/query-tests/Security Features/CWE-937/packages.config @@ -0,0 +1,10 @@ + + + + + + + + + + From ff3ed2db18b6224f5a07929261c0e4e35f10fe3c Mon Sep 17 00:00:00 2001 From: calum Date: Wed, 17 Oct 2018 13:19:01 +0100 Subject: [PATCH 02/22] C#: Autobuilder extracts XML for .csproj and .props files. --- .../Semmle.Autobuild.Tests/BuildScripts.cs | 56 ++++++++++++++----- .../Semmle.Autobuild/XmlBuildRule.cs | 10 +++- 2 files changed, 50 insertions(+), 16 deletions(-) diff --git a/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs b/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs index d49493c55480..8cad94c9ecca 100644 --- a/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs +++ b/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs @@ -357,6 +357,8 @@ public void TestDefaultCSharpAutoBuilder() Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -364,7 +366,7 @@ public void TestDefaultCSharpAutoBuilder() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", true); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 8); } [Fact] @@ -376,6 +378,8 @@ public void TestLinuxCSharpAutoBuilder() Actions.RunProcess[@"C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -383,7 +387,7 @@ public void TestLinuxCSharpAutoBuilder() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 8); } [Fact] @@ -487,6 +491,8 @@ public void TestLinuxBuildlessExtractionSuccess() Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone --references:."] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -494,7 +500,7 @@ public void TestLinuxBuildlessExtractionSuccess() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless:"true"); - TestAutobuilderScript(autobuilder, 0, 3); + TestAutobuilderScript(autobuilder, 0, 5); } [Fact] @@ -519,6 +525,8 @@ public void TestLinuxBuildlessExtractionSolution() Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone foo.sln --references:."] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -526,7 +534,7 @@ public void TestLinuxBuildlessExtractionSolution() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless: "true", solution: "foo.sln"); - TestAutobuilderScript(autobuilder, 0, 3); + TestAutobuilderScript(autobuilder, 0, 5); } void SkipVsWhere() @@ -563,6 +571,8 @@ public void TestLinuxBuildCommand() Actions.RunProcess["C:\\odasa\\tools\\odasa index --auto \"./build.sh --skip-tests\""] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -572,7 +582,7 @@ public void TestLinuxBuildCommand() SkipVsWhere(); var autobuilder = CreateAutoBuilder("csharp", false, buildCommand:"./build.sh --skip-tests"); - TestAutobuilderScript(autobuilder, 0, 3); + TestAutobuilderScript(autobuilder, 0, 5); } [Fact] @@ -588,10 +598,12 @@ public void TestLinuxBuildSh() Actions.RunProcessWorkingDirectory[@"C:\odasa\tools\odasa index --auto build/build.sh"] = "build"; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 5); + TestAutobuilderScript(autobuilder, 0, 7); } [Fact] @@ -642,10 +654,12 @@ public void TestWindowsBuildBat() Actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\odasa\tools\odasa index --auto build.bat"] = ""; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", true); - TestAutobuilderScript(autobuilder, 0, 4); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -693,6 +707,8 @@ public void TestWindowCSharpMsBuild() Actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false; @@ -712,7 +728,7 @@ public void TestWindowCSharpMsBuild() autobuilder.SolutionsToBuild.Add(testSolution1); autobuilder.SolutionsToBuild.Add(testSolution2); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 8); } [Fact] @@ -749,6 +765,8 @@ public void TestSkipNugetMsBuild() Actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false; @@ -768,7 +786,7 @@ public void TestSkipNugetMsBuild() autobuilder.SolutionsToBuild.Add(testSolution1); autobuilder.SolutionsToBuild.Add(testSolution2); - TestAutobuilderScript(autobuilder, 0, 4); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -777,6 +795,8 @@ public void TestSkipNugetBuildless() Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone foo.sln --references:. --skip-nuget"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -784,7 +804,7 @@ public void TestSkipNugetBuildless() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless: "true", solution: "foo.sln", nugetRestore:"false"); - TestAutobuilderScript(autobuilder, 0, 3); + TestAutobuilderScript(autobuilder, 0, 5); } @@ -797,6 +817,8 @@ public void TestSkipNugetDotnet() Actions.RunProcess[@"C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false --no-restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -804,7 +826,7 @@ public void TestSkipNugetDotnet() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetArguments:"--no-restore"); // nugetRestore=false does not work for now. - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 8); } [Fact] @@ -821,6 +843,8 @@ public void TestDotnetVersionNotInstalled() Actions.RunProcess[@"C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -829,7 +853,7 @@ public void TestDotnetVersionNotInstalled() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion:"2.1.3"); - TestAutobuilderScript(autobuilder, 0, 10); + TestAutobuilderScript(autobuilder, 0, 12); } [Fact] @@ -846,6 +870,8 @@ public void TestDotnetVersionAlreadyInstalled() Actions.RunProcess[@"C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -854,7 +880,7 @@ public void TestDotnetVersionAlreadyInstalled() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 10); + TestAutobuilderScript(autobuilder, 0, 12); } [Fact] @@ -869,6 +895,8 @@ public void TestDotnetVersionWindows() Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -877,7 +905,7 @@ public void TestDotnetVersionWindows() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", true, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 8); + TestAutobuilderScript(autobuilder, 0, 10); } } } diff --git a/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs b/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs index 6b0a5b1cbb19..b2ba3bf3b084 100644 --- a/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs +++ b/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs @@ -9,10 +9,16 @@ class XmlBuildRule : IBuildRule { public BuildScript Analyse(Autobuilder builder) { - var command = new CommandBuilder(builder.Actions). + var config = new CommandBuilder(builder.Actions). RunCommand(builder.Odasa). Argument("index --xml --extensions config"); - return command.Script; + var csproj = new CommandBuilder(builder.Actions). + RunCommand(builder.Odasa). + Argument("index --xml --extensions csproj"); + var props = new CommandBuilder(builder.Actions). + RunCommand(builder.Odasa). + Argument("index --xml --extensions props"); + return config.Script & csproj.Script & props.Script; } } } From 5ad060c1beea6f414ac7a51c22a68abc4b3bb79d Mon Sep 17 00:00:00 2001 From: calum Date: Wed, 17 Oct 2018 20:25:06 +0100 Subject: [PATCH 03/22] C#: qhelp for VulnerablePackage. --- .../CWE-937/Vulnerabilities.qll | 1 - .../CWE-937/VulnerablePackage.qhelp | 43 +++++++++++++++++++ .../CWE-937/VulnerablePackageBAD.csproj | 15 +++++++ .../CWE-937/VulnerablePackageGOOD.csproj | 15 +++++++ 4 files changed, 73 insertions(+), 1 deletion(-) create mode 100644 csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp create mode 100644 csharp/ql/src/Security Features/CWE-937/VulnerablePackageBAD.csproj create mode 100644 csharp/ql/src/Security Features/CWE-937/VulnerablePackageGOOD.csproj diff --git a/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll index 39b94e599088..9e69fa16f00a 100644 --- a/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll +++ b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll @@ -11,7 +11,6 @@ import csharp import Vulnerability class MicrosoftAdvisory4021279 extends Vulnerability { - MicrosoftAdvisory4021279() { this = "Microsoft Security Advisory 4021279" } override string getUrl() { result = "https://github.com/dotnet/corefx/issues/19535" } diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp new file mode 100644 index 000000000000..b75522282edb --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp @@ -0,0 +1,43 @@ + + + + +

+Using a package with a known vulnerability is a security risk that could leave the +software vulnerable to attack. +

+

+This query reads the packages imported by the project build files and +.config files, and checks them against a list of packages with known +vulnerabilities. +

+ + + +

+Upgrade the package to the recommended version, for example using the NuGet package manager, +or by editing the project files directly. +

+ + + +

+The following example shows a C# project file referencing package System.Net.Http +version 4.3.1, which is vulnerable to CVE-2018-8292. +

+ +

+The project file can be fixed by changing the version of the package to 4.3.4. +

+ + + + +
  • +OWASP: A9-Using Components with Known Vulnerabilities. +
    • +     + +     diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackageBAD.csproj b/csharp/ql/src/Security Features/CWE-937/VulnerablePackageBAD.csproj new file mode 100644 index 000000000000..b13494984ecb --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackageBAD.csproj @@ -0,0 +1,15 @@ + + + + netcoreapp2.0 + Semmle.Autobuild + Semmle.Autobuild + Exe + + + + + + + + diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackageGOOD.csproj b/csharp/ql/src/Security Features/CWE-937/VulnerablePackageGOOD.csproj new file mode 100644 index 000000000000..98275b86a6fc --- /dev/null +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackageGOOD.csproj @@ -0,0 +1,15 @@ + + + + netcoreapp2.0 + Semmle.Autobuild + Semmle.Autobuild + Exe + + + + + + + + From ee396af3856e26273916accb6aca5d357bf79112 Mon Sep 17 00:00:00 2001 From: calum Date: Wed, 17 Oct 2018 20:30:52 +0100 Subject: [PATCH 04/22] C#: Update analysis change notes. --- change-notes/1.19/analysis-csharp.md | 3 ++- csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/change-notes/1.19/analysis-csharp.md b/change-notes/1.19/analysis-csharp.md index 3b85b8f367c0..5a9e506a4eb0 100644 --- a/change-notes/1.19/analysis-csharp.md +++ b/change-notes/1.19/analysis-csharp.md @@ -8,7 +8,8 @@ | **Query** | **Tags** | **Purpose** | |-----------------------------|-----------|--------------------------------------------------------------------| -| *@name of query (Query ID)* | *Tags* |*Aim of the new query and whether it is enabled by default or not* | +| Using a package with a known vulnerability (cs/use-of-vulnerable-package) | security, external/cwe/cwe-937 | Finds project build files that import packages with known vulnerabilities. This is included by default. | + ## Changes to existing queries diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql index a1803d488a9e..f5b5fc5ba87c 100644 --- a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql @@ -1,5 +1,5 @@ /** - * @name Using a package with a known vulnerability. + * @name Using a package with a known vulnerability * @description Using a package with a known vulnerability is a security risk. * Upgrade the package to a version that does not contain the vulnerability. * @kind problem From 6e96fba7f61a73e8e7034a8b0c5fff9ada0234ae Mon Sep 17 00:00:00 2001 From: calum Date: Fri, 19 Oct 2018 16:14:35 +0100 Subject: [PATCH 05/22] C#: Address review comments: Merge XML index commands. --- .../Semmle.Autobuild.Tests/BuildScripts.cs | 84 +++++++------------ .../Semmle.Autobuild/XmlBuildRule.cs | 12 +-- 2 files changed, 31 insertions(+), 65 deletions(-) diff --git a/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs b/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs index 8cad94c9ecca..1979dcdf5b9c 100644 --- a/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs +++ b/csharp/extractor/Semmle.Autobuild.Tests/BuildScripts.cs @@ -356,9 +356,7 @@ public void TestDefaultCSharpAutoBuilder() Actions.RunProcess["cmd.exe /C dotnet restore"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -366,7 +364,7 @@ public void TestDefaultCSharpAutoBuilder() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", true); - TestAutobuilderScript(autobuilder, 0, 8); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -377,9 +375,7 @@ public void TestLinuxCSharpAutoBuilder() Actions.RunProcess["dotnet restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -387,7 +383,7 @@ public void TestLinuxCSharpAutoBuilder() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 8); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -490,9 +486,7 @@ public void TestLinuxBuildlessExtractionSuccess() { Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone --references:."] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -500,7 +494,7 @@ public void TestLinuxBuildlessExtractionSuccess() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless:"true"); - TestAutobuilderScript(autobuilder, 0, 5); + TestAutobuilderScript(autobuilder, 0, 3); } [Fact] @@ -524,9 +518,7 @@ public void TestLinuxBuildlessExtractionSolution() { Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone foo.sln --references:."] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -534,7 +526,7 @@ public void TestLinuxBuildlessExtractionSolution() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless: "true", solution: "foo.sln"); - TestAutobuilderScript(autobuilder, 0, 5); + TestAutobuilderScript(autobuilder, 0, 3); } void SkipVsWhere() @@ -570,9 +562,7 @@ public void TestLinuxBuildCommand() { Actions.RunProcess["C:\\odasa\\tools\\odasa index --auto \"./build.sh --skip-tests\""] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -582,7 +572,7 @@ public void TestLinuxBuildCommand() SkipVsWhere(); var autobuilder = CreateAutoBuilder("csharp", false, buildCommand:"./build.sh --skip-tests"); - TestAutobuilderScript(autobuilder, 0, 5); + TestAutobuilderScript(autobuilder, 0, 3); } [Fact] @@ -597,13 +587,11 @@ public void TestLinuxBuildSh() Actions.RunProcess[@"C:\odasa\tools\odasa index --auto build/build.sh"] = 0; Actions.RunProcessWorkingDirectory[@"C:\odasa\tools\odasa index --auto build/build.sh"] = "build"; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", false); - TestAutobuilderScript(autobuilder, 0, 7); + TestAutobuilderScript(autobuilder, 0, 5); } [Fact] @@ -653,13 +641,11 @@ public void TestWindowsBuildBat() Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto build.bat"] = 0; Actions.RunProcessWorkingDirectory[@"cmd.exe /C C:\odasa\tools\odasa index --auto build.bat"] = ""; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; var autobuilder = CreateAutoBuilder("csharp", true); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 4); } [Fact] @@ -706,9 +692,7 @@ public void TestWindowCSharpMsBuild() Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\csharp\nuget\nuget.exe restore C:\Project\test2.sln"] = 0; Actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false; @@ -728,7 +712,7 @@ public void TestWindowCSharpMsBuild() autobuilder.SolutionsToBuild.Add(testSolution1); autobuilder.SolutionsToBuild.Add(testSolution2); - TestAutobuilderScript(autobuilder, 0, 8); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -764,9 +748,7 @@ public void TestSkipNugetMsBuild() Actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test1.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0; Actions.RunProcess["cmd.exe /C CALL ^\"C:\\Program Files ^(x86^)\\Microsoft Visual Studio 12.0\\VC\\vcvarsall.bat^\" && C:\\odasa\\tools\\odasa index --auto msbuild C:\\Project\\test2.sln /p:UseSharedCompilation=false /t:Windows /p:Platform=\"x86\" /p:Configuration=\"Debug\" /p:MvcBuildViews=true /P:Fu=Bar"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe"] = false; Actions.FileExists[@"C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\vcvarsall.bat"] = false; @@ -786,7 +768,7 @@ public void TestSkipNugetMsBuild() autobuilder.SolutionsToBuild.Add(testSolution1); autobuilder.SolutionsToBuild.Add(testSolution2); - TestAutobuilderScript(autobuilder, 0, 6); + TestAutobuilderScript(autobuilder, 0, 4); } [Fact] @@ -794,9 +776,7 @@ public void TestSkipNugetBuildless() { Actions.RunProcess[@"C:\odasa\tools\csharp\Semmle.Extraction.CSharp.Standalone foo.sln --references:. --skip-nuget"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -804,7 +784,7 @@ public void TestSkipNugetBuildless() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, buildless: "true", solution: "foo.sln", nugetRestore:"false"); - TestAutobuilderScript(autobuilder, 0, 5); + TestAutobuilderScript(autobuilder, 0, 3); } @@ -816,9 +796,7 @@ public void TestSkipNugetDotnet() Actions.RunProcess["dotnet restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --auto dotnet build --no-incremental /p:UseSharedCompilation=false --no-restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -826,7 +804,7 @@ public void TestSkipNugetDotnet() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetArguments:"--no-restore"); // nugetRestore=false does not work for now. - TestAutobuilderScript(autobuilder, 0, 8); + TestAutobuilderScript(autobuilder, 0, 6); } [Fact] @@ -842,9 +820,7 @@ public void TestDotnetVersionNotInstalled() Actions.RunProcess[@"C:\Project\.dotnet\dotnet restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -853,7 +829,7 @@ public void TestDotnetVersionNotInstalled() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion:"2.1.3"); - TestAutobuilderScript(autobuilder, 0, 12); + TestAutobuilderScript(autobuilder, 0, 10); } [Fact] @@ -869,9 +845,7 @@ public void TestDotnetVersionAlreadyInstalled() Actions.RunProcess[@"C:\Project\.dotnet\dotnet restore"] = 0; Actions.RunProcess[@"C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -880,7 +854,7 @@ public void TestDotnetVersionAlreadyInstalled() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", false, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 12); + TestAutobuilderScript(autobuilder, 0, 10); } [Fact] @@ -894,9 +868,7 @@ public void TestDotnetVersionWindows() Actions.RunProcess[@"cmd.exe /C C:\Project\.dotnet\dotnet restore"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --auto C:\Project\.dotnet\dotnet build --no-incremental /p:UseSharedCompilation=false"] = 0; Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\java\bin\java -jar C:\odasa\tools\extractor-asp.jar ."] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions csproj"] = 0; - Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions props"] = 0; + Actions.RunProcess[@"cmd.exe /C C:\odasa\tools\odasa index --xml --extensions config csproj props xml"] = 0; Actions.FileExists["csharp.log"] = true; Actions.GetEnvironmentVariable["TRAP_FOLDER"] = null; Actions.GetEnvironmentVariable["SOURCE_ARCHIVE"] = null; @@ -905,7 +877,7 @@ public void TestDotnetVersionWindows() Actions.EnumerateDirectories[@"C:\Project"] = ""; var autobuilder = CreateAutoBuilder("csharp", true, dotnetVersion: "2.1.3"); - TestAutobuilderScript(autobuilder, 0, 10); + TestAutobuilderScript(autobuilder, 0, 8); } } } diff --git a/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs b/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs index b2ba3bf3b084..420fc508bfb7 100644 --- a/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs +++ b/csharp/extractor/Semmle.Autobuild/XmlBuildRule.cs @@ -9,16 +9,10 @@ class XmlBuildRule : IBuildRule { public BuildScript Analyse(Autobuilder builder) { - var config = new CommandBuilder(builder.Actions). + var command = new CommandBuilder(builder.Actions). RunCommand(builder.Odasa). - Argument("index --xml --extensions config"); - var csproj = new CommandBuilder(builder.Actions). - RunCommand(builder.Odasa). - Argument("index --xml --extensions csproj"); - var props = new CommandBuilder(builder.Actions). - RunCommand(builder.Odasa). - Argument("index --xml --extensions props"); - return config.Script & csproj.Script & props.Script; + Argument("index --xml --extensions config csproj props xml"); + return command.Script; } } } From 61232cb08e387c51d51dff386a39ae238cac5d30 Mon Sep 17 00:00:00 2001 From: calum Date: Fri, 19 Oct 2018 16:33:04 +0100 Subject: [PATCH 06/22] C#: Address review comments in QL. --- .../Security Features/CWE-937/Vulnerabilities.qll | 8 +++++--- .../src/Security Features/CWE-937/Vulnerability.qll | 8 ++++---- .../Security Features/CWE-937/VulnerablePackage.ql | 2 +- .../CWE-937/VulnerablePackage.expected | 12 ++++++------ 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll index 9e69fa16f00a..13e2ea301bbc 100644 --- a/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll +++ b/csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll @@ -1,5 +1,5 @@ /** - * A list of NuGet packages with known vulnerabilities. + * Provides a list of NuGet packages with known vulnerabilities. * * To add a new vulnerability follow the existing pattern. * Create a new class that extends the abstract class `Vulnerability`, @@ -113,11 +113,13 @@ class AspNetCore_Mar18 extends Vulnerability { name = "Microsoft.AspNetCore.Server.Kestrel.Transport.Abstractions" or name = "Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv" - or - name = "Microsoft.AspNetCore.All" ) and affected = "2.0.0" and fixed = "2.0.3" + or + name = "Microsoft.AspNetCore.All" and + affected = "2.0.0" and + fixed = "2.0.8" } } diff --git a/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll b/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll index 4a58a95efe97..2c8063e1b668 100644 --- a/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll +++ b/csharp/ql/src/Security Features/CWE-937/Vulnerability.qll @@ -2,7 +2,7 @@ import csharp /** * A package reference in an XML file, for example in a - * .csproj file, a .props file or a packages.config file. + * `.csproj` file, a `.props` file, or a `packages.config` file. */ class Package extends XMLElement { string name; @@ -43,16 +43,16 @@ abstract class Vulnerability extends string { Vulnerability() { any() } /** - * A package with name `name` is vulnerable from version `affected` + * Holds if a package with name `name` is vulnerable from version `affected` * until version `fixed`. */ predicate matchesRange(string name, Version affected, Version fixed) { none() } /** - * A package with name `name` is vulnerable in version `affected`, and + * Holds if a package with name `name` is vulnerable in version `affected`, and * is fixed by version `fixed`. */ - predicate matchesVersion(string name, Version affecter, Version fixed) { none() } + predicate matchesVersion(string name, Version affected, Version fixed) { none() } /** Gets the URL describing the vulnerability. */ abstract string getUrl(); diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql index f5b5fc5ba87c..956317935076 100644 --- a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.ql @@ -15,5 +15,5 @@ import Vulnerabilities from Vulnerability vuln, VulnerablePackage package where vuln = package.getVulnerability() -select package, "Package " + package + " has vulnerability $@, and should be upgraded to version " + package.getFixedVersion() + ".", +select package, "Package '" + package + "' has vulnerability $@, and should be upgraded to version " + package.getFixedVersion() + ".", vuln.getUrl(), vuln.toString() diff --git a/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected index b088930b7eb2..bd39de4e295a 100644 --- a/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected +++ b/csharp/ql/test/query-tests/Security Features/CWE-937/VulnerablePackage.expected @@ -1,6 +1,6 @@ -| csproj.config:10:5:10:77 | System.Text.Encodings.Web 4.3.0 | Package System.Text.Encodings.Web 4.3.0 has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | -| csproj.config:11:5:11:75 | system.text.encodings.web 4.3 | Package system.text.encodings.web 4.3 has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | -| csproj.config:12:5:12:67 | System.Net.Http 4.1.1 | Package System.Net.Http 4.1.1 has vulnerability $@, and should be upgraded to version 4.1.2. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | -| csproj.config:13:5:13:67 | System.Net.Http 4.1.2 | Package System.Net.Http 4.1.2 has vulnerability $@, and should be upgraded to version 4.3.4. | https://github.com/dotnet/announcements/issues/88 | CVE-2018-8292 | -| packages.config:8:3:8:79 | System.IO.Pipelines 4.5.0 | Package System.IO.Pipelines 4.5.0 has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | -| packages.config:9:3:9:81 | System.IO.Pipelines 4.5.0.0 | Package System.IO.Pipelines 4.5.0.0 has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | +| csproj.config:10:5:10:77 | System.Text.Encodings.Web 4.3.0 | Package 'System.Text.Encodings.Web 4.3.0' has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:11:5:11:75 | system.text.encodings.web 4.3 | Package 'system.text.encodings.web 4.3' has vulnerability $@, and should be upgraded to version 4.3.1. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:12:5:12:67 | System.Net.Http 4.1.1 | Package 'System.Net.Http 4.1.1' has vulnerability $@, and should be upgraded to version 4.1.2. | https://github.com/dotnet/corefx/issues/19535 | Microsoft Security Advisory 4021279 | +| csproj.config:13:5:13:67 | System.Net.Http 4.1.2 | Package 'System.Net.Http 4.1.2' has vulnerability $@, and should be upgraded to version 4.3.4. | https://github.com/dotnet/announcements/issues/88 | CVE-2018-8292 | +| packages.config:8:3:8:79 | System.IO.Pipelines 4.5.0 | Package 'System.IO.Pipelines 4.5.0' has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | +| packages.config:9:3:9:81 | System.IO.Pipelines 4.5.0.0 | Package 'System.IO.Pipelines 4.5.0.0' has vulnerability $@, and should be upgraded to version 4.5.1. | https://github.com/aspnet/Announcements/issues/316 | CVE-2018-8409 | From fde3341455adcab6ded69f8cf03492dfe28e7441 Mon Sep 17 00:00:00 2001 From: calum Date: Thu, 25 Oct 2018 14:18:30 +0100 Subject: [PATCH 07/22] C#: Addressed documentation review. --- csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp index b75522282edb..112642c7958d 100644 --- a/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp +++ b/csharp/ql/src/Security Features/CWE-937/VulnerablePackage.qhelp @@ -17,7 +17,7 @@ vulnerabilities.

    -Upgrade the package to the recommended version, for example using the NuGet package manager, +Upgrade the package to the recommended version using, for example, the NuGet package manager, or by editing the project files directly.

    From 73b186a6952f0d12e40e5de4a9ea5bf34a2a3bf7 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 7 Nov 2018 13:33:18 +0000 Subject: [PATCH 08/22] CPP: Add test case. --- .../ExprHasNoEffect/ExprHasNoEffect.expected | 2 ++ .../Likely Typos/ExprHasNoEffect/template.cpp | 22 +++++++++++++++++++ 2 files changed, 24 insertions(+) create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected index 20cc25fb99b1..379943d85823 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected @@ -1,5 +1,7 @@ | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 | | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 | +| template.cpp:4:3:4:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | +| template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | | test.c:7:5:7:5 | 0 | This expression has no effect. | test.c:7:5:7:5 | 0 | | | test.c:9:8:9:8 | 1 | This expression has no effect. | test.c:9:8:9:8 | 1 | | | test.c:9:11:9:11 | 2 | This expression has no effect. | test.c:9:11:9:11 | 2 | | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp new file mode 100644 index 000000000000..d0a922a3f981 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp @@ -0,0 +1,22 @@ + +template +void Increment(T &t) { + t++; // GOOD (sometimes has an effect) [FALSE POSITIVE] +} + +class Nothing { +public: + Nothing operator++(int) { + return *this; // do nothing + } +}; + +void myTemplateTest() { + int i = 0; + Nothing n; + + i++; // GOOD (always has an effect) + n++; // BAD (never has an effect) + Increment(i); + Increment(n); +} From 7bf9200a18b1da9045e0d2d96ed3d0a43daf9eb5 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 7 Nov 2018 14:12:52 +0000 Subject: [PATCH 09/22] CPP: Fix (it looks like we already had a similar test, both are fixed. --- cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql | 2 +- .../Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected | 6 ------ .../Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp | 2 +- .../Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp | 2 +- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql b/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql index 9e42cb5d2ea0..56715956635f 100644 --- a/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql +++ b/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql @@ -99,7 +99,7 @@ where // EQExprs are covered by CompareWhereAssignMeant.ql not peivc.(FunctionCall).getTarget().hasName("operator==") and not accessInInitOfForStmt(peivc) and not peivc.isCompilerGenerated() and - not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and + not peivc.isFromTemplateInstantiation(_) and parent = peivc.getParent() and not parent.isInMacroExpansion() and not parent instanceof PureExprInVoidContext and diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected index 379943d85823..f50c352fe340 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected @@ -1,6 +1,5 @@ | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 | | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 | -| template.cpp:4:3:4:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | | template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | | test.c:7:5:7:5 | 0 | This expression has no effect. | test.c:7:5:7:5 | 0 | | | test.c:9:8:9:8 | 1 | This expression has no effect. | test.c:9:8:9:8 | 1 | | @@ -21,11 +20,6 @@ | test.c:26:15:26:16 | 32 | This expression has no effect. | test.c:26:15:26:16 | 32 | | | test.c:27:9:27:10 | 33 | This expression has no effect. | test.c:27:9:27:10 | 33 | | | test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | -| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | -| test.cpp:24:3:24:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | -| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | -| test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | | test.cpp:25:3:25:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | -| test.cpp:26:3:26:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | test.cpp:9:14:9:23 | operator++ | operator++ | | test.cpp:62:5:62:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:47:14:47:22 | operator= | operator= | | test.cpp:65:5:65:5 | call to operator= | This expression has no effect (because $@ has no external side effects). | test.cpp:55:7:55:7 | operator= | operator= | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp index d0a922a3f981..ecc3d6246034 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/template.cpp @@ -1,7 +1,7 @@ template void Increment(T &t) { - t++; // GOOD (sometimes has an effect) [FALSE POSITIVE] + t++; // GOOD (sometimes has an effect) } class Nothing { diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp index 33235b5c4454..1658f210cf3b 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/test.cpp @@ -23,7 +23,7 @@ class MyTemplateClass ++arg1; // pure, does nothing ++arg2; // pure, does nothing - ++arg3; // not pure in all cases (when _It is int this has a side-effect) [FALSE POSITIVE] + ++arg3; // not pure in all cases (when _It is int this has a side-effect) return arg2; } From 5f12c188df15a9a5cf1ac690ff75cf4c39c57730 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 7 Nov 2018 14:19:12 +0000 Subject: [PATCH 10/22] CPP: Change note. --- change-notes/1.19/analysis-cpp.md | 1 + 1 file changed, 1 insertion(+) diff --git a/change-notes/1.19/analysis-cpp.md b/change-notes/1.19/analysis-cpp.md index 4835afd0c70d..60e038cf557e 100644 --- a/change-notes/1.19/analysis-cpp.md +++ b/change-notes/1.19/analysis-cpp.md @@ -16,6 +16,7 @@ | **Query** | **Expected impact** | **Change** | |----------------------------|------------------------|------------------------------------------------------------------| | Empty branch of conditional | Fewer false positive results | The query now recognizes commented blocks more reliably. | +| Expression has no effect | Fewer false positive results | Expressions in template instantiations are now excluded from this query. | | Resource not released in destructor | Fewer false positive results | Placement new is now excluded from the query. Also fixed an issue where false positives could occur if the destructor body was not in the snapshot. | | Missing return statement (`cpp/missing-return`) | Visible by default | The precision of this query has been increased from 'medium' to 'high', which makes it visible by default in LGTM. It was 'medium' in release 1.17 and 1.18 because it had false positives due to an extractor bug that was fixed in 1.18. | | Missing return statement | Fewer false positive results | The query is now produces correct results when a function returns a template-dependent type. | From d6f27f0b2d51decaed5219fe5972ebe6daba6a11 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 8 Nov 2018 09:55:39 +0000 Subject: [PATCH 11/22] CPP: Add a test of macros. --- .../Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected | 1 + .../Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c | 7 +++++++ 2 files changed, 8 insertions(+) create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected index f50c352fe340..9341c913f822 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected @@ -1,3 +1,4 @@ +| macros.c:6:9:6:13 | param | This expression has no effect. | macros.c:6:9:6:13 | param | | | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 | | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 | | template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c new file mode 100644 index 000000000000..9a8a922e4184 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c @@ -0,0 +1,7 @@ + +#define UNUSED(x) (x) + +void test2(int param) +{ + UNUSED(param); // GOOD [FALSE POSITIVE] +} From 5b09e11a5224a95e9b55085fc8fd02a22352a2c3 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Thu, 8 Nov 2018 10:01:07 +0000 Subject: [PATCH 12/22] CPP: Repair macro case. --- cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql | 1 + .../Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected | 1 - .../Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql b/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql index 56715956635f..2db4616a08a1 100644 --- a/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql +++ b/cpp/ql/src/Likely Bugs/Likely Typos/ExprHasNoEffect.ql @@ -99,6 +99,7 @@ where // EQExprs are covered by CompareWhereAssignMeant.ql not peivc.(FunctionCall).getTarget().hasName("operator==") and not accessInInitOfForStmt(peivc) and not peivc.isCompilerGenerated() and + not exists(Macro m | peivc = m.getAnInvocation().getAnExpandedElement()) and not peivc.isFromTemplateInstantiation(_) and parent = peivc.getParent() and not parent.isInMacroExpansion() and diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected index 9341c913f822..f50c352fe340 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/ExprHasNoEffect.expected @@ -1,4 +1,3 @@ -| macros.c:6:9:6:13 | param | This expression has no effect. | macros.c:6:9:6:13 | param | | | preproc.c:89:2:89:4 | call to fn4 | This expression has no effect (because $@ has no external side effects). | preproc.c:33:5:33:7 | fn4 | fn4 | | preproc.c:94:2:94:4 | call to fn9 | This expression has no effect (because $@ has no external side effects). | preproc.c:78:5:78:7 | fn9 | fn9 | | template.cpp:19:3:19:3 | call to operator++ | This expression has no effect (because $@ has no external side effects). | template.cpp:9:10:9:19 | operator++ | operator++ | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c index 9a8a922e4184..2f7fcc1e05e2 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c +++ b/cpp/ql/test/query-tests/Likely Bugs/Likely Typos/ExprHasNoEffect/macros.c @@ -3,5 +3,5 @@ void test2(int param) { - UNUSED(param); // GOOD [FALSE POSITIVE] + UNUSED(param); // GOOD } From 55f4839abfd0588c3d6e260763ab2604f5bfb3bd Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Tue, 6 Nov 2018 15:39:29 -0800 Subject: [PATCH 13/22] Allow mixed whitespace in JavaScript test sources --- javascript/ql/test/format.json | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 javascript/ql/test/format.json diff --git a/javascript/ql/test/format.json b/javascript/ql/test/format.json new file mode 100644 index 000000000000..fbfbbc417894 --- /dev/null +++ b/javascript/ql/test/format.json @@ -0,0 +1,6 @@ +[ + { + "pattern": "**/*.js", + "allowMixedTabsAndSpaces": true + } +] From a141f4c81ab7d02173a991fe9a535d39eb50b544 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Tue, 6 Nov 2018 16:00:25 -0800 Subject: [PATCH 14/22] Allow mixed whitespace in C#, C++, and Java test sources --- cpp/ql/test/format.json | 11 +++++++++++ csharp/ql/test/format.json | 6 ++++++ java/ql/test/format.json | 6 ++++++ 3 files changed, 23 insertions(+) create mode 100644 cpp/ql/test/format.json create mode 100644 csharp/ql/test/format.json create mode 100644 java/ql/test/format.json diff --git a/cpp/ql/test/format.json b/cpp/ql/test/format.json new file mode 100644 index 000000000000..2dc62f0b7205 --- /dev/null +++ b/cpp/ql/test/format.json @@ -0,0 +1,11 @@ +[ + { + "pattern": [ + "**/*.c", + "**/*.cpp", + "**/*.h", + "**/*.hpp" + ], + "allowMixedTabsAndSpaces": true + } +] diff --git a/csharp/ql/test/format.json b/csharp/ql/test/format.json new file mode 100644 index 000000000000..d334c08c26a1 --- /dev/null +++ b/csharp/ql/test/format.json @@ -0,0 +1,6 @@ +[ + { + "pattern": "**/*.cs", + "allowMixedTabsAndSpaces": true + } +] diff --git a/java/ql/test/format.json b/java/ql/test/format.json new file mode 100644 index 000000000000..f51e422c7c6a --- /dev/null +++ b/java/ql/test/format.json @@ -0,0 +1,6 @@ +[ + { + "pattern": "**/*.java", + "allowMixedTabsAndSpaces": true + } +] From d521502dedbeb7490edea4d4c778e5a914c777f1 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 7 Nov 2018 16:08:43 -0800 Subject: [PATCH 15/22] Allow mixed whitespace in parser tests --- javascript/extractor/.project | 12 ++++++------ javascript/extractor/format.json | 10 ++++++++++ 2 files changed, 16 insertions(+), 6 deletions(-) create mode 100644 javascript/extractor/format.json diff --git a/javascript/extractor/.project b/javascript/extractor/.project index b653bbd47da8..1c021b3d5af8 100644 --- a/javascript/extractor/.project +++ b/javascript/extractor/.project @@ -5,13 +5,13 @@ - - org.eclipse.jdt.core.javabuilder - - - + + org.eclipse.jdt.core.javabuilder + + + - org.eclipse.jdt.core.javanature + org.eclipse.jdt.core.javanature diff --git a/javascript/extractor/format.json b/javascript/extractor/format.json new file mode 100644 index 000000000000..069da4db954b --- /dev/null +++ b/javascript/extractor/format.json @@ -0,0 +1,10 @@ +[ + { + "pattern": [ + "lib/**/*.js", + "parser-tests/**/*.js", + "tests/**/*.js" + ], + "allowMixedTabsAndSpaces": true + } +] From 2977395c327a1fff5d3e0f3b0412f4485a6a9cb6 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 7 Nov 2018 16:55:15 -0800 Subject: [PATCH 16/22] Ignore whitespace errors in everything under lib --- javascript/extractor/format.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/extractor/format.json b/javascript/extractor/format.json index 069da4db954b..b4fc0e651c72 100644 --- a/javascript/extractor/format.json +++ b/javascript/extractor/format.json @@ -1,7 +1,7 @@ [ { "pattern": [ - "lib/**/*.js", + "lib/**/*", "parser-tests/**/*.js", "tests/**/*.js" ], From bdfe938d02a36e56fffc1362b0c1d8eb0da489fb Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Wed, 31 Oct 2018 10:32:42 -0400 Subject: [PATCH 17/22] JavaScript: Improve `StackTraceExposure` query. It now also flags exposure of the entire exception object (not just the `stack` property). --- change-notes/1.19/analysis-javascript.md | 1 + .../security/dataflow/StackTraceExposure.qll | 22 ++++++++++++++----- .../CWE-209/StackTraceExposure.expected | 4 +++- .../test/query-tests/Security/CWE-209/tst.js | 18 +++++++++++++++ 4 files changed, 38 insertions(+), 7 deletions(-) create mode 100644 javascript/ql/test/query-tests/Security/CWE-209/tst.js diff --git a/change-notes/1.19/analysis-javascript.md b/change-notes/1.19/analysis-javascript.md index cca00ae7da8f..118e7fa0239d 100644 --- a/change-notes/1.19/analysis-javascript.md +++ b/change-notes/1.19/analysis-javascript.md @@ -44,6 +44,7 @@ | Unused import | Fewer false-positive results | This rule no longer flags imports used by the `transform-react-jsx` Babel plugin. | | Self assignment | Fewer false-positive results | This rule now ignores self-assignments preceded by a JSDoc comment with a `@type` tag. | | Client side cross-site scripting | More results | This rule now also flags HTML injection in the body of an email. | +| Information exposure through a stack trace | More results | This rule now also flags cases where the entire exception object (including the stack trace) may be exposed. | ## Changes to QL libraries diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/StackTraceExposure.qll b/javascript/ql/src/semmle/javascript/security/dataflow/StackTraceExposure.qll index 75579403dd20..e0367666a8d1 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/StackTraceExposure.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/StackTraceExposure.qll @@ -23,22 +23,32 @@ module StackTraceExposure { src instanceof Source } + override predicate isSanitizer(DataFlow::Node nd) { + super.isSanitizer(nd) + or + // read of a property other than `stack` + nd.(DataFlow::PropRead).getPropertyName() != "stack" + or + // `toString` does not include the stack trace + nd.(DataFlow::MethodCallNode).getMethodName() = "toString" + or + nd = StringConcatenation::getAnOperand(_) + } + override predicate isSink(DataFlow::Node snk) { snk instanceof Sink } } + /** * A read of the `stack` property of an exception, viewed as a data flow * sink for stack trace exposure vulnerabilities. */ - class DefaultSource extends Source, DataFlow::ValueNode { + class DefaultSource extends Source, DataFlow::Node { DefaultSource() { - // any read of the `stack` property of an exception is a source - exists (Parameter exc | - exc = any(TryStmt try).getACatchClause().getAParameter() and - this = DataFlow::parameterNode(exc).getAPropertyRead("stack") - ) + // any exception is a source + this = DataFlow::parameterNode(any(TryStmt try).getACatchClause().getAParameter()) } } diff --git a/javascript/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected b/javascript/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected index 8f89e63b419a..8ec5b31e6e9d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected +++ b/javascript/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected @@ -1 +1,3 @@ -| node.js:11:13:11:21 | err.stack | Stack trace information from $@ may be exposed to an external user here. | node.js:11:13:11:21 | err.stack | here | +| node.js:11:13:11:21 | err.stack | Stack trace information from $@ may be exposed to an external user here. | node.js:8:10:8:12 | err | here | +| tst.js:7:13:7:13 | e | Stack trace information from $@ may be exposed to an external user here. | tst.js:6:12:6:12 | e | here | +| tst.js:17:11:17:17 | e.stack | Stack trace information from $@ may be exposed to an external user here. | tst.js:6:12:6:12 | e | here | diff --git a/javascript/ql/test/query-tests/Security/CWE-209/tst.js b/javascript/ql/test/query-tests/Security/CWE-209/tst.js new file mode 100644 index 000000000000..b1450ce088ae --- /dev/null +++ b/javascript/ql/test/query-tests/Security/CWE-209/tst.js @@ -0,0 +1,18 @@ +var http = require('http'); + +http.createServer(function onRequest(req, res) { + try { + throw new Error(); + } catch (e) { + res.end(e); // NOT OK + fail(res, e); + res.end(e.message); // OK + res.end("Caught exception " + e); // OK + res.end(e.toString()); // OK + res.end(`Caught exception ${e}.`); // OK + } +}); + +function fail(res, e) { + res.end(e.stack); // NOT OK +} From fa8736adbc6a5b66b562d9c5055dff9ac880ab1f Mon Sep 17 00:00:00 2001 From: Max Schaefer Date: Fri, 9 Nov 2018 11:26:49 +0000 Subject: [PATCH 18/22] JavaScript: Introduce aliases for compatibility with other language libraries. --- javascript/ql/src/javascript.qll | 1 + .../ql/src/semmle/javascript/Aliases.qll | 37 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 javascript/ql/src/semmle/javascript/Aliases.qll diff --git a/javascript/ql/src/javascript.qll b/javascript/ql/src/javascript.qll index faa1a83b0cfb..030bc3ce33d3 100644 --- a/javascript/ql/src/javascript.qll +++ b/javascript/ql/src/javascript.qll @@ -2,6 +2,7 @@ * Provides classes for working with JavaScript programs, as well as JSON, YAML and HTML. */ +import semmle.javascript.Aliases import semmle.javascript.AMD import semmle.javascript.AST import semmle.javascript.BasicBlocks diff --git a/javascript/ql/src/semmle/javascript/Aliases.qll b/javascript/ql/src/semmle/javascript/Aliases.qll new file mode 100644 index 000000000000..2e8d394e227c --- /dev/null +++ b/javascript/ql/src/semmle/javascript/Aliases.qll @@ -0,0 +1,37 @@ +/** + * Provides aliases for commonly used classes that have different names + * in the QL libraries for other languages. + */ + +import javascript + +class AndBitwiseExpr = BitAndExpr; +class AndLogicalExpr = LogAndExpr; +class ArrayAccess = IndexExpr; +class AssignOp = CompoundAssignExpr; +class Block = BlockStmt; +class BoolLiteral = BooleanLiteral; +class CaseStmt = Case; +class ComparisonOperation = Comparison; +class DoStmt = DoWhileStmt; +class EqualityOperation = EqualityTest; +class FieldAccess = DotExpr; +class InstanceOfExpr = InstanceofExpr; +class LabelStmt = LabeledStmt; +class LogicalAndExpr = LogAndExpr; +class LogicalNotExpr = LogNotExpr; +class LogicalOrExpr = LogOrExpr; +class Loop = LoopStmt; +class MultilineComment = BlockComment; +class OrBitwiseExpr = BitOrExpr; +class OrLogicalExpr = LogOrExpr; +class ParenthesisExpr = ParExpr; +class ParenthesizedExpr = ParExpr; +class RelationalOperation = RelationalComparison; +class RemExpr = ModExpr; +class SingleLineComment = LineComment; +class SuperAccess = SuperExpr; +class SwitchCase = Case; +class ThisAccess = ThisExpr; +class VariableAccess = VarAccess; +class XorBitwiseExpr = XOrExpr; From d5c6f4fd6421a41432bf030692c67f4ac63f46f5 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 9 Nov 2018 11:31:44 +0000 Subject: [PATCH 19/22] CPP: Correct typo in OverflowCalculated.cpp example. --- cpp/ql/src/Critical/OverflowCalculated.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/src/Critical/OverflowCalculated.cpp b/cpp/ql/src/Critical/OverflowCalculated.cpp index 8e9b7511283c..00a6a47b4070 100644 --- a/cpp/ql/src/Critical/OverflowCalculated.cpp +++ b/cpp/ql/src/Critical/OverflowCalculated.cpp @@ -1,5 +1,5 @@ void f(char* string) { - // wrong: allocates space for characters, put not zero terminator + // wrong: allocates space for characters, but not zero terminator char* buf = malloc(strlen(string)); strcpy(buf, string); From e645166feecae2ace3bcd50c513469e40fc6bbf4 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 9 Nov 2018 11:36:39 +0000 Subject: [PATCH 20/22] CPP: Make InconsistentNullnessTest.cpp example plausible. --- cpp/ql/src/Critical/InconsistentNullnessTesting.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp b/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp index b071e79ecdb8..d7d60b46806c 100644 --- a/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp +++ b/cpp/ql/src/Critical/InconsistentNullnessTesting.cpp @@ -1,10 +1,10 @@ void* f() { - block = malloc(BLOCK_SIZE); + block = (MyBlock *)malloc(sizeof(MyBlock)); if (block) { //correct: block is checked for nullness here block->id = NORMAL_BLOCK_ID; } //... /* make sure data-portion is null-terminated */ - block[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here + block->data[BLOCK_SIZE - 1] = '\0'; //wrong: block not checked for nullness here return block; } From 3f0e28aea9139f4f19799f21e83c20e548a39dcf Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 9 Nov 2018 17:12:51 +0000 Subject: [PATCH 21/22] CPP: Fix additional expr_has_no_effect test. --- .../queries/expr_has_no_effect/expr_has_no_effect.expected | 1 - 1 file changed, 1 deletion(-) diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected b/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected index af2bacade1a2..5982cfa589ef 100644 --- a/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected +++ b/cpp/ql/test/library-tests/queries/expr_has_no_effect/expr_has_no_effect.expected @@ -1,6 +1,5 @@ | calls.cpp:8:5:8:5 | 1 | This expression has no effect. | calls.cpp:8:5:8:5 | 1 | | | calls.cpp:12:5:12:16 | call to thingy | This expression has no effect (because $@ has no external side effects). | calls.cpp:7:15:7:20 | thingy | thingy | -| templatey.cpp:4:3:4:8 | ... << ... | This expression has no effect. | templatey.cpp:4:3:4:8 | ... << ... | | | templatey.cpp:39:3:39:23 | call to pointless_add_numbers | This expression has no effect (because $@ has no external side effects). | templatey.cpp:29:5:29:25 | pointless_add_numbers | pointless_add_numbers | | volatile.c:9:5:9:5 | c | This expression has no effect. | volatile.c:9:5:9:5 | c | | | volatile.c:12:5:12:9 | access to array | This expression has no effect. | volatile.c:12:5:12:9 | access to array | | From 09782d145eabdf83ed69821b75b4cadc5e3c0da7 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 9 Nov 2018 17:23:32 +0000 Subject: [PATCH 22/22] CPP: Annotate expr_has_no_effect test. --- .../queries/expr_has_no_effect/calls.cpp | 8 ++++---- .../queries/expr_has_no_effect/templatey.cpp | 12 ++++++------ .../queries/expr_has_no_effect/volatile.c | 10 +++++----- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp b/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp index d9a71fa7bafa..2acdfcf80f8f 100644 --- a/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp +++ b/cpp/ql/test/library-tests/queries/expr_has_no_effect/calls.cpp @@ -5,11 +5,11 @@ int external(); class Base { public: virtual int thingy() { - 1; + 1; // BAD } int our_thingy() { - Base::thingy(); + Base::thingy(); // BAD return 2; } }; @@ -17,14 +17,14 @@ class Base { class Derived : public Base { public: virtual int thingy() { - external(); + external(); // GOOD return 3; } }; void internal() { Base* ptr = new Derived(); - ptr->thingy(); + ptr->thingy(); // GOOD } } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp b/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp index 114e2052a530..7d2b6b19777e 100644 --- a/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp +++ b/cpp/ql/test/library-tests/queries/expr_has_no_effect/templatey.cpp @@ -1,7 +1,7 @@ template void foo(T x, T y) { - x << y; + x << y; // GOOD (effect depends on T) }; struct streamable @@ -15,9 +15,9 @@ void operator<<(streamable& lhs, streamable& rhs) int main() { int x = 3; - foo(x, x); + foo(x, x); // BAD [NOT DETECTED] streamable y; - foo(y, y); + foo(y, y); // BAD [NOT DETECTED] return 0; } @@ -34,7 +34,7 @@ int pointless_add_numbers(int lhs, int rhs) void call_add_numbers() { int accum = 0; - add_numbers(accum, 4); - add_numbers(accum, 10); - pointless_add_numbers(accum, 20); + add_numbers(accum, 4); // GOOD + add_numbers(accum, 10); // GOOD + pointless_add_numbers(accum, 20); // BAD } diff --git a/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c b/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c index 27ee532c99c2..c34e0818f192 100644 --- a/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c +++ b/cpp/ql/test/library-tests/queries/expr_has_no_effect/volatile.c @@ -6,18 +6,18 @@ char *pc; volatile char *pv; void f(void) { - c; - v; + c; // BAD + v; // (accesses to volatile variables are considered impure) - pc[5]; + pc[5]; // BAD pv[5]; ((volatile char *)pc)[5]; - *pc; + *pc; // BAD *pv; *((volatile char *)pc); - *(pc + 5); + *(pc + 5); // BAD *(pv + 5); *((volatile char *)(pc + 5)); *(((volatile char *)pc) + 5);