Skip to content

C#: Cache NamedElement::getLabel() #674

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
calumgrant merged 3 commits into from
Dec 17, 2018
Merged

C#: Cache NamedElement::getLabel() #674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1366638
C#: Fix whitespaces
hvitved Dec 12, 2018
6918dad
C#: Refactor `localFlowStep()`
hvitved Dec 12, 2018
74167e4
C#: Cache `NamedElement::getLabel()`
hvitved Dec 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions csharp/ql/src/Likely Bugs/Collections/ContainerLengthCmpOffByOne.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ import semmle.code.csharp.commons.ComparisonTest
class IndexGuard extends ComparisonTest {
VariableAccess indexAccess;
Variable array;

IndexGuard() {
this.getFirstArgument() = indexAccess and
this.getSecondArgument() = any(PropertyAccess lengthAccess |
this.getSecondArgument() = any(PropertyAccess lengthAccess |
lengthAccess.getQualifier() = array.getAnAccess() and
lengthAccess.getTarget().hasName("Length")
)
Expand All @@ -50,7 +50,7 @@ from IndexGuard incorrectGuard, Variable array, Variable index, ElementAccess ea
where
// Look for `index <= array.Length` or `array.Length >= index`
incorrectGuard.controls(array, index) and
incorrectGuard.isIncorrect() and
incorrectGuard.isIncorrect() and
// Look for `array[index]`
ea.getQualifier() = array.getAnAccess() and
ea.getIndex(0) = indexAccess and
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/src/Security Features/CWE-730/ReDoS.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ from TaintTrackingConfiguration c, DataFlow::PathNode source, DataFlow::PathNode
where
c.hasFlowPath(source, sink) and
// No global timeout set
not exists(RegexGlobalTimeout r) and
not exists(RegexGlobalTimeout r) and
(
sink.getNode() instanceof Sink
or
Expand Down
22 changes: 11 additions & 11 deletions csharp/ql/src/Security Features/CWE-937/Vulnerabilities.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
* 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`,
* supplying the name and the URL, and override one (or both) of
Expand Down Expand Up @@ -73,9 +73,9 @@ class MicrosoftAdvisory4021279 extends Vulnerability {

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"
Expand All @@ -91,9 +91,9 @@ class CVE_2017_8700 extends Vulnerability {

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
Expand All @@ -103,7 +103,7 @@ class CVE_2018_0765 extends Vulnerability {

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) {
Expand All @@ -125,9 +125,9 @@ class AspNetCore_Mar18 extends Vulnerability {

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
Expand All @@ -138,9 +138,9 @@ class CVE_2018_8409 extends Vulnerability {

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"
Expand Down Expand Up @@ -204,7 +204,7 @@ class CVE_2018_8356 extends Vulnerability {

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) {
Expand Down
4 changes: 2 additions & 2 deletions csharp/ql/src/semmle/code/csharp/controlflow/Guards.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ class DereferenceableExpr extends Expr {
ie = any(IsTypeExpr ite | ite.getCheckedType() = ite.getExpr().getType()) and
branch = false and
isNull = true
)
)
)
or
this.hasNullableType() and
Expand Down Expand Up @@ -1189,7 +1189,7 @@ module Internal {
g1 = cond and
v1 = v.getDualValue() and
(
// g1 === g2 ? e : ...;
// g1 === g2 ? e : ...;
g2 = cond.getCondition() and
v2 = TBooleanValue(branch.booleanNot())
or
Expand Down
18 changes: 7 additions & 11 deletions csharp/ql/src/semmle/code/csharp/dataflow/DataFlow.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ module DataFlow {
localFlowStep*(source, sink)
}

predicate localFlowStep = localFlowStepCached/2;
predicate localFlowStep = Internal::LocalFlow::step/2;

/**
* A data flow node augmented with a call context and a configuration. Only
Expand Down Expand Up @@ -690,12 +690,14 @@ module DataFlow {
/**
* Provides predicates related to local data flow.
*/
private module LocalFlow {
module LocalFlow {
/**
* Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
* (intra-procedural) step.
*/
predicate localFlowStepNonCached(Node nodeFrom, Node nodeTo) {
cached
predicate step(Node nodeFrom, Node nodeTo) {
forceCachingInSameStage() and
localFlowStepExpr(nodeFrom.asExpr(), nodeTo.asExpr())
or
// Flow from source to SSA definition
Expand Down Expand Up @@ -1119,6 +1121,8 @@ module DataFlow {
* same stage.
*/
cached module Cached {
cached predicate forceCachingInSameStage() { any() }

cached newtype TNode =
TExprNode(DotNet::Expr e)
or
Expand All @@ -1137,14 +1141,6 @@ module DataFlow {
)
}

/**
* Holds if data flows from `nodeFrom` to `nodeTo` in exactly one local
* (intra-procedural) step.
*/
cached predicate localFlowStepCached(Node nodeFrom, Node nodeTo) {
LocalFlow::localFlowStepNonCached(nodeFrom, nodeTo)
}

/**
* Holds if `pred` can flow to `succ`, by jumping from one callable to
* another.
Expand Down
1 change: 1 addition & 0 deletions csharp/ql/src/semmle/code/dotnet/Element.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ class NamedElement extends Element, @dotnet_named_element {
}

/** Gets a unique string label for this element. */
cached
string getLabel() { none() }

/**
Expand Down
2 changes: 1 addition & 1 deletion csharp/ql/test/library-tests/cil/regressions/Methods.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* This tests the correct extraction of F<T>, and we should end up with
* 2 constructed methods of F<T>.
*/

// semmle-extractor-options: --cil

namespace Methods
Expand Down
6 changes: 3 additions & 3 deletions ...ry-tests/Likely Bugs/Collections/ContainerLengthCmpOffByOne/ContainerLengthCmpOffByOne.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ void Test2(string[] args)
}

void Test3(string[] args)
{
{
// GOOD: Guarded by ternary operator.
for (int i = 0; i <= args.Length; i++)
{
Expand All @@ -68,7 +68,7 @@ void Test4(string[] args)
}

void Test5(string[] args)
{
{
// GOOD: A valid test of Length.
for (int i = 0; i != args.Length; i++)
{
Expand All @@ -94,6 +94,6 @@ void Test7(string[] args)
for (int i = 0; i <= args.Length; i++)
{
bool b = i == args.Length || args[i] == "x";
}
}
}
}
4 changes: 2 additions & 2 deletions csharp/ql/test/query-tests/Security Features/CWE-937/csproj.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@
<PackageReference Include="System.Net.Http" Version="4.3.4" />
<PackageReference Include="System.Text.Encodings.Web" Version="4.2.9" />
<PackageReference Include="System.Text.Encodings.Web" Version="4.3.1" />

<!-- These are BAD -->
<PackageReference Include="System.Text.Encodings.Web" Version="4.3.0" />
<PackageReference Include="system.text.encodings.web" Version="4.3" />
<PackageReference Include="System.Net.Http" Version="4.1.1" />
<PackageReference Include="System.Net.Http" Version="4.1.2" />

</ItemGroup>
</Project>
2 changes: 1 addition & 1 deletion csharp/ql/test/query-tests/Security Features/CWE-937/packages.config
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<package id="System.IO.Pipelines" version="4.5.1" targetFramework="net45" />
<package id="System.IO.Pipelines" version="4.5.1.0" targetFramework="net45" />
<package id="Microsoft.AspNetCore.All" version="2.0.9" targetFramework="net45" />

<!-- These are BAD -->
<package id="System.IO.Pipelines" version="4.5.0" targetFramework="net45" />
<package id="System.IO.Pipelines" version="4.5.0.0" targetFramework="net45" />
Expand Down