Skip to content
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.

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

C#: Add VS Code model editor queries #14200

Merged
merged 22 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
585fb9d
C#: Add VS Code model editor queries
koesie10 Sep 13, 2023
0cc74a2
C#: Extract TestLibrary to separate module
koesie10 Sep 18, 2023
e524e35
C#: Check accessor declaration for publicness
koesie10 Sep 18, 2023
ff2cef3
C#: Switch from Declaration to Callable
koesie10 Sep 18, 2023
8472b84
C#: Remove unnecessary isEffectivelyPublic predicate
koesie10 Sep 18, 2023
f468b2a
C#: Add tests for generic interfaces/classes/methods
koesie10 Sep 18, 2023
4693f72
C#: Rename CallableMethod to Endpoint
koesie10 Sep 18, 2023
93972a4
C#: Rename AutomodelVsCode to ModelEditor
koesie10 Sep 18, 2023
81a8eee
C#: Only include unbound declarations in endpoints
koesie10 Sep 18, 2023
948e36a
C#: Update comment for Endpoint
koesie10 Sep 18, 2023
489561f
C#: Fix formatting of ExternalApi
koesie10 Sep 18, 2023
dd79049
C#: Remove unnecessary isUnboundDeclaration predicates
koesie10 Sep 19, 2023
14a2b7f
C#: Add tests for private methods and accessors
koesie10 Sep 19, 2023
eace7a4
C#: Add tests for supported framework methods
koesie10 Sep 19, 2023
044fb9f
C#: Rename queries from fetch methods to endpoints
koesie10 Sep 19, 2023
3ebb9e1
C#: Update query id/tags and documentation
koesie10 Sep 19, 2023
45432f2
C#: Identify whether callables in the source code are supported in te…
michaelnebel Sep 20, 2023
50a9219
C#: Re-factor most of the logic out of the model editor query files.
michaelnebel Sep 20, 2023
13dd9a6
C#: Address review comments.
michaelnebel Sep 20, 2023
0fea21f
C#: Remove unnecessary columns
koesie10 Sep 22, 2023
dc6def7
C#: Split API name column into separate columns
koesie10 Sep 25, 2023
922ff7b
C#: Remove unnecessary import
koesie10 Sep 25, 2023
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
176 changes: 176 additions & 0 deletions csharp/ql/src/utils/modeleditor/AutomodelVsCode.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/** Provides classes and predicates related to handling APIs for the VS Code extension. */

private import csharp
private import dotnet
private import semmle.code.csharp.dispatch.Dispatch
private import semmle.code.csharp.dataflow.ExternalFlow
private import semmle.code.csharp.dataflow.FlowSummary
private import semmle.code.csharp.dataflow.internal.DataFlowImplCommon as DataFlowImplCommon
private import semmle.code.csharp.dataflow.internal.DataFlowPrivate
private import semmle.code.csharp.dataflow.internal.DataFlowDispatch as DataFlowDispatch
private import semmle.code.csharp.dataflow.internal.FlowSummaryImpl as FlowSummaryImpl
private import semmle.code.csharp.dataflow.internal.TaintTrackingPrivate
private import semmle.code.csharp.frameworks.Test
private import semmle.code.csharp.security.dataflow.flowsources.Remote

pragma[nomagic]
private predicate isTestNamespace(Namespace ns) {
ns.getFullName()
.matches([
"NUnit.Framework%", "Xunit%", "Microsoft.VisualStudio.TestTools.UnitTesting%", "Moq%"
])
}

/**
* A test library.
*/
class TestLibrary extends RefType {
TestLibrary() { isTestNamespace(this.getNamespace()) }
}
michaelnebel marked this conversation as resolved.
Show resolved Hide resolved

/** Holds if the given callable is not worth supporting. */
private predicate isUninteresting(DotNet::Declaration c) {
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
c.getDeclaringType() instanceof TestLibrary or
c.(Constructor).isParameterless() or
c.getDeclaringType() instanceof AnonymousClass
}

/**
* An callable method from either the C# Standard Library, a 3rd party library, or from the source.
*/
class CallableMethod extends DotNet::Declaration {
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
michaelnebel marked this conversation as resolved.
Show resolved Hide resolved
CallableMethod() {
michaelnebel marked this conversation as resolved.
Show resolved Hide resolved
this.(Modifiable).isEffectivelyPublic() and
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
not isUninteresting(this)
}

/**
* Gets the unbound type, name and parameter types of this API.
*/
bindingset[this]
private string getSignature() {
result =
nestedName(this.getDeclaringType().getUnboundDeclaration()) + "#" + this.getName() + "(" +
parameterQualifiedTypeNamesToString(this) + ")"
}

/**
* Gets the namespace of this API.
*/
bindingset[this]
string getNamespace() { this.getDeclaringType().hasQualifiedName(result, _) }

/**
* Gets the namespace and signature of this API.
*/
bindingset[this]
string getApiName() { result = this.getNamespace() + "." + this.getSignature() }

private string getDllName() { result = this.getLocation().(Assembly).getName() }

private string getDllVersion() { result = this.getLocation().(Assembly).getVersion().toString() }

string dllName() {
result = this.getDllName()
or
not exists(this.getDllName()) and result = this.getFile().getBaseName()
}

string dllVersion() {
result = this.getDllVersion()
or
not exists(this.getDllVersion()) and result = ""
}

/** Gets a node that is an input to a call to this API. */
private ArgumentNode getAnInput() {
result
.getCall()
.(DataFlowDispatch::NonDelegateDataFlowCall)
.getATarget(_)
.getUnboundDeclaration() = this
michaelnebel marked this conversation as resolved.
Show resolved Hide resolved
}

/** Gets a node that is an output from a call to this API. */
private DataFlow::Node getAnOutput() {
exists(
Call c, DataFlowDispatch::NonDelegateDataFlowCall dc, DataFlowImplCommon::ReturnKindExt ret
|
dc.getDispatchCall().getCall() = c and
c.getTarget().getUnboundDeclaration() = this
michaelnebel marked this conversation as resolved.
Show resolved Hide resolved
|
result = ret.getAnOutNode(dc)
)
}

/** Holds if this API has a supported summary. */
pragma[nomagic]
predicate hasSummary() {
this instanceof SummarizedCallable
or
defaultAdditionalTaintStep(this.getAnInput(), _)
}

/** Holds if this API is a known source. */
pragma[nomagic]
predicate isSource() {
this.getAnOutput() instanceof RemoteFlowSource or sourceNode(this.getAnOutput(), _)
}

/** Holds if this API is a known sink. */
pragma[nomagic]
predicate isSink() { sinkNode(this.getAnInput(), _) }

/** Holds if this API is a known neutral. */
pragma[nomagic]
predicate isNeutral() { this instanceof FlowSummaryImpl::Public::NeutralCallable }

/**
* Holds if this API is supported by existing CodeQL libraries, that is, it is either a
* recognized source, sink or neutral or it has a flow summary.
*/
predicate isSupported() {
this.hasSummary() or this.isSource() or this.isSink() or this.isNeutral()
}
}

boolean isSupported(CallableMethod callableMethod) {
callableMethod.isSupported() and result = true
or
not callableMethod.isSupported() and
result = false
}

string supportedType(CallableMethod method) {
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
method.isSink() and result = "sink"
or
method.isSource() and result = "source"
or
method.hasSummary() and result = "summary"
or
method.isNeutral() and result = "neutral"
or
not method.isSupported() and result = ""
}

string methodClassification(Call method) {
method.getFile() instanceof TestFile and result = "test"
or
not method.getFile() instanceof TestFile and
result = "source"
}

/**
* Gets the nested name of the declaration.
*
* If the declaration is not a nested type, the result is the same as `getName()`.
* Otherwise the name of the nested type is prefixed with a `+` and appended to
* the name of the enclosing type, which might be a nested type as well.
*/
private string nestedName(Declaration declaration) {
not exists(declaration.getDeclaringType().getUnboundDeclaration()) and
result = declaration.getName()
or
nestedName(declaration.getDeclaringType().getUnboundDeclaration()) + "+" + declaration.getName() =
result
}
32 changes: 32 additions & 0 deletions csharp/ql/src/utils/modeleditor/FetchApplicationModeMethods.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* @name Fetch model editor methods (application mode)
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
* @description A list of 3rd party APIs used in the codebase. Excludes test and generated code.
* @kind problem
* @problem.severity recommendation
* @id csharp/utils/modeleditor/fetch-application-mode-methods
* @tags modeleditor fetch methods application-mode
*/

private import csharp
private import AutomodelVsCode

class ExternalApi extends CallableMethod {
ExternalApi() {
this.isUnboundDeclaration() and
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
this.fromLibrary() and
this.(Modifiable).isEffectivelyPublic()
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
}
}

private Call aUsage(ExternalApi api) { result.getTarget().getUnboundDeclaration() = api }

from
ExternalApi api, string apiName, boolean supported, Call usage, string type, string classification
where
apiName = api.getApiName() and
supported = isSupported(api) and
usage = aUsage(api) and
type = supportedType(api) and
classification = methodClassification(usage)
select usage, apiName, supported.toString(), "supported", api.dllName(), api.dllVersion(), type,
"type", classification, "classification"
25 changes: 25 additions & 0 deletions csharp/ql/src/utils/modeleditor/FetchFrameworkModeMethods.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* @name Fetch model editor methods (framework mode)
* @description A list of APIs callable by consumers. Excludes test and generated code.
* @kind problem
* @problem.severity recommendation
* @id csharp/utils/modeleditor/fetch-framework-mode-methods
* @tags modeleditor fetch methods framework-mode
*/

private import csharp
private import dotnet
private import semmle.code.csharp.frameworks.Test
private import AutomodelVsCode

class PublicMethod extends CallableMethod {
PublicMethod() { this.fromSource() and not this.getFile() instanceof TestFile }
}

from PublicMethod publicMethod, string apiName, boolean supported, string type
where
apiName = publicMethod.getApiName() and
supported = isSupported(publicMethod) and
type = supportedType(publicMethod)
select publicMethod, apiName, supported.toString(), "supported",
publicMethod.getFile().getBaseName(), "library", type, "type", "unknown", "classification"
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
| NonPublicClass.cs:9:9:9:31 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicClass.cs:9:9:9:30 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicClass.cs:14:9:14:30 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicClass.cs:19:9:19:51 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System.Console#ReadLine() | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicClass.cs:19:33:19:50 | call to method ReadLine | System.Console#ReadLine() | true | supported | System.Console | 7.0.0.0 | source | type | source | classification |
| PublicClass.cs:24:9:24:30 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
| PublicInterface.cs:11:9:11:30 | call to method WriteLine | System.Console#WriteLine(System.String) | true | supported | System.Console | 7.0.0.0 | neutral | type | source | classification |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
utils/modeleditor/FetchApplicationModeMethods.ql
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
| PublicClass.cs:7:17:7:21 | stuff | GitHub.CodeQL.PublicClass#stuff(System.String) | false | supported | PublicClass.cs | library | | type | unknown | classification |
| PublicClass.cs:12:24:12:34 | staticStuff | GitHub.CodeQL.PublicClass#staticStuff(System.String) | false | supported | PublicClass.cs | library | | type | unknown | classification |
| PublicClass.cs:17:20:17:33 | nonPublicStuff | GitHub.CodeQL.PublicClass#nonPublicStuff(System.String) | false | supported | PublicClass.cs | library | | type | unknown | classification |
| PublicInterface.cs:7:10:7:14 | stuff | GitHub.CodeQL.PublicInterface#stuff(System.String) | false | supported | PublicInterface.cs | library | | type | unknown | classification |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the callables are reported to be not supported as there doesn't exist any models for them.
Maybe consider to add some positive testcases for this as well.
It is possible to add a data extension file that only applies to an individual test case (codeql test run looks for .ext.yml files with the same name as the test case). That is, if you add a file called FetchFrameworkModeMethods.ext.yml you can add models for some of your testmethods.

This will implicitly help in testing that we got the predicates for isSupported right (e.g. getAnInput and getAnOutput). The ApplicationMode testcase primarily targets "source" models.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion! I've added 4 new methods with the 4 different types of models. The summaryModel and neutralModel seem to work, but I can't get the sinkModel or sourceModel to work. Would you be able to take a look and check why those are not working?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the problem is that the implementation requires that for a callable to be identified as a source or sink there needs to be an actual call to it (this works fine for the queries that are just interested in callables NOT the source code). We need to do something else for the analysis that should respect source and sink models for EndPoints in the source code (I will look into this).

| PublicInterface.cs:9:17:9:27 | staticStuff | GitHub.CodeQL.PublicInterface#staticStuff(System.String) | false | supported | PublicInterface.cs | library | | type | unknown | classification |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
utils/modeleditor/FetchFrameworkModeMethods.ql
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
11 changes: 11 additions & 0 deletions csharp/ql/test/utils/modeleditor/NonPublicClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
using System;

namespace GitHub.CodeQL;

class NonPublicClass
{
public void noCandidates(String here)
{
Console.WriteLine(here);
}
}
26 changes: 26 additions & 0 deletions csharp/ql/test/utils/modeleditor/PublicClass.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
using System;
koesie10 marked this conversation as resolved.
Show resolved Hide resolved

namespace GitHub.CodeQL;

public class PublicClass
{
public void stuff(String arg)
{
Console.WriteLine(arg);
}

public static void staticStuff(String arg)
{
Console.WriteLine(arg);
}

protected void nonPublicStuff(String arg)
koesie10 marked this conversation as resolved.
Show resolved Hide resolved
{
Console.WriteLine(arg + Console.ReadLine());
}

internal void internalStuff(String arg)
{
Console.WriteLine(arg);
}
}
13 changes: 13 additions & 0 deletions csharp/ql/test/utils/modeleditor/PublicInterface.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
using System;

namespace GitHub.CodeQL;

public interface PublicInterface
{
void stuff(String arg);

static void staticStuff(String arg)
{
Console.WriteLine(arg);
}
}
1 change: 1 addition & 0 deletions misc/suite-helpers/code-scanning-selectors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@
- /Diagnostics/Internal/.*/
- exclude:
tags contain:
- modeleditor
- modelgenerator
1 change: 1 addition & 0 deletions misc/suite-helpers/security-and-quality-selectors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,5 @@
- /Diagnostics/Internal/.*/
- exclude:
tags contain:
- modeleditor
- modelgenerator
1 change: 1 addition & 0 deletions misc/suite-helpers/security-experimental-selectors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@
- /Diagnostics/Internal/.*/
- exclude:
tags contain:
- modeleditor
- model-generator
1 change: 1 addition & 0 deletions misc/suite-helpers/security-extended-selectors.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,5 @@
- /Diagnostics/Internal/.*/
- exclude:
tags contain:
- modeleditor
- modelgenerator
Loading