Skip to content

Commit

Permalink
Merge pull request #16861 from michaelnebel/modelgen/sourcesinklift
Browse files Browse the repository at this point in the history
C#/Java: Do not lift source and sink models.
  • Loading branch information
michaelnebel committed Jul 2, 2024
2 parents b4707ab + 9cb7018 commit 25b2018
Show file tree
Hide file tree
Showing 17 changed files with 183 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class DataFlowSourceTargetApi = SourceTargetApi;
class DataFlowSinkTargetApi = SinkTargetApi;

private module ModelPrintingInput implements ModelPrintingSig {
class Api = TargetApiBase;
class SummaryApi = DataFlowSummaryTargetApi;

class SourceOrSinkApi = SourceOrSinkTargetApi;

string getProvenance() { result = "df-generated" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,38 +112,41 @@ predicate isUninterestingForDataFlowModels(CS::Callable api) { isHigherOrder(api
predicate isUninterestingForTypeBasedFlowModels(CS::Callable api) { none() }

/**
* A class of callables that are potentially relevant for generating summary and
* neutral models.
* A class of callables that are potentially relevant for generating source or
* sink models.
*/
class SummaryTargetApi extends TargetApiBase {
SummaryTargetApi() { not hasManualSummaryModel(this.lift()) }
class SourceOrSinkTargetApi extends Callable {
SourceOrSinkTargetApi() { relevant(this) }
}

/**
* A class of callables that are potentially relevant for generating sink models.
*/
class SinkTargetApi extends TargetApiBase {
SinkTargetApi() { not hasManualSinkModel(this.lift()) }
class SinkTargetApi extends SourceOrSinkTargetApi {
SinkTargetApi() { not hasManualSinkModel(this) }
}

/**
* A class of callables that are potentially relevant for generating source models.
*/
class SourceTargetApi extends TargetApiBase {
SourceTargetApi() { not hasManualSourceModel(this.lift()) }
class SourceTargetApi extends SourceOrSinkTargetApi {
SourceTargetApi() { not hasManualSourceModel(this) }
}

/**
* A class of callables that are potentially relevant for generating summary, source, sink
* and neutral models.
* A class of callables that are potentially relevant for generating summary or
* neutral models.
*
* In the Standard library and 3rd party libraries it is the callables (or callables that have a
* super implementation) that can be called from outside the library itself.
*/
class TargetApiBase extends Callable {
class SummaryTargetApi extends Callable {
private Callable lift;

TargetApiBase() { lift = liftedImpl(this) }
SummaryTargetApi() {
lift = liftedImpl(this) and
not hasManualSummaryModel(lift)
}

/**
* Gets the callable that a model will be lifted to.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ private predicate output(Callable callable, TypeParameter tp, string output) {
}

private module ModelPrintingInput implements ModelPrintingSig {
class Api = TypeBasedFlowTargetApi;
class SummaryApi = TypeBasedFlowTargetApi;

class SourceOrSinkApi = TypeBasedFlowTargetApi;

string getProvenance() { result = "tb-generated" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ extensions:
extensible: sinkModel
data:
- [ "Sinks", "NewSinks", False, "Sink", "(System.Object)", "", "Argument[0]", "test-sink", "manual"]
- [ "Sinks", "NewSinks", False, "Sink2", "(System.Object)", "", "Argument[0]", "test-sink2", "manual"]
- [ "Sinks", "NewSinks", False, "ManualSinkAlreadyDefined", "(System.Object)", "", "Argument[0]", "test-sink", "manual"]

- addsTo:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ extensions:
extensible: sourceModel
data:
- ["Sources", "NewSources", False, "ManualSourceAlreadyDefined", "()", "", "ReturnValue", "test-source", "manual"]
- ["Sources", "NewSources", False, "Source1", "()", "", "ReturnValue", "source-kind-1", "manual"]
- ["Sources", "NewSources", False, "Source2", "()", "", "ReturnValue", "source-kind-2", "manual"]

- addsTo:
pack: codeql/csharp-all
Expand Down
32 changes: 31 additions & 1 deletion csharp/ql/test/utils/modelgenerator/dataflow/Sinks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@ public class NewSinks

// Sink defined in the extensible file next to the test.
// neutral=Sinks;NewSinks;Sink;(System.Object);summary;df-generated
public void Sink(object o) => throw null;
public static void Sink(object o) => throw null;

// Sink defined in the extensible file next to the test.
// neutral=Sinks;NewSinks;Sink2;(System.Object);summary;df-generated
public static void Sink2(object o) => throw null;

// New sink
// sink=Sinks;NewSinks;false;WrapResponseWrite;(System.Object);;Argument[0];html-injection;df-generated
Expand Down Expand Up @@ -105,6 +109,32 @@ public void ManualSinkAlreadyDefined(object o)
{
Sink(o);
}

public abstract class DataWriter
{
// neutral=Sinks;NewSinks+DataWriter;Write;(System.Object);summary;df-generated
public abstract void Write(object o);
}

public class DataWriterKind1 : DataWriter
{
// sink=Sinks;NewSinks+DataWriterKind1;true;Write;(System.Object);;Argument[0];test-sink;df-generated
// neutral=Sinks;NewSinks+DataWriterKind1;Write;(System.Object);summary;df-generated
public override void Write(object o)
{
Sink(o);
}
}

public class DataWriterKind2 : DataWriter
{
// sink=Sinks;NewSinks+DataWriterKind2;true;Write;(System.Object);;Argument[0];test-sink2;df-generated
// neutral=Sinks;NewSinks+DataWriterKind2;Write;(System.Object);summary;df-generated
public override void Write(object o)
{
Sink2(o);
}
}
}

public class CompoundSinks
Expand Down
35 changes: 35 additions & 0 deletions csharp/ql/test/utils/modelgenerator/dataflow/Sources.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,15 @@ namespace Sources;

public class NewSources
{
// Defined as source in the extensions file next to the test.
// neutral=Sources;NewSources;Source1;();summary;df-generated
public static string Source1() => throw null;

// Defined as source in the extensions file next to the test.
// neutral=Sources;NewSources;Source2;();summary;df-generated
public static string Source2() => throw null;


// New source
// source=Sources;NewSources;false;WrapConsoleReadLine;();;ReturnValue;local;df-generated
// neutral=Sources;NewSources;WrapConsoleReadLine;();summary;df-generated
Expand Down Expand Up @@ -79,4 +88,30 @@ public string ManualSourceAlreadyDefined()
{
return Console.ReadLine();
}

public abstract class DataReader
{
// neutral=Sources;NewSources+DataReader;Read;();summary;df-generated
public abstract string Read();
}

public class DataReaderKind1 : DataReader
{
// source=Sources;NewSources+DataReaderKind1;true;Read;();;ReturnValue;source-kind-1;df-generated
// neutral=Sources;NewSources+DataReaderKind1;Read;();summary;df-generated
public override string Read()
{
return Source1();
}
}

public class DataReaderKind2 : DataReader
{
// source=Sources;NewSources+DataReaderKind2;true;Read;();;ReturnValue;source-kind-2;df-generated
// neutral=Sources;NewSources+DataReaderKind2;Read;();summary;df-generated
public override string Read()
{
return Source2();
}
}
}
4 changes: 3 additions & 1 deletion java/ql/src/utils/modelgenerator/internal/CaptureModels.qll
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ class DataFlowSourceTargetApi = SourceTargetApi;
class DataFlowSinkTargetApi = SinkTargetApi;

private module ModelPrintingInput implements ModelPrintingSig {
class Api = TargetApiBase;
class SummaryApi = DataFlowSummaryTargetApi;

class SourceOrSinkApi = SourceOrSinkTargetApi;

string getProvenance() { result = "df-generated" }
}
Expand Down
27 changes: 15 additions & 12 deletions java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll
Original file line number Diff line number Diff line change
Expand Up @@ -83,25 +83,25 @@ predicate isUninterestingForDataFlowModels(Callable api) {
}

/**
* A class of callables that are potentially relevant for generating summary and
* neutral models.
* A class of callables that are potentially relevant for generating source or
* sink models.
*/
class SummaryTargetApi extends TargetApiBase {
SummaryTargetApi() { not hasManualSummaryModel(this.lift()) }
class SourceOrSinkTargetApi extends Callable {
SourceOrSinkTargetApi() { relevant(this) }
}

/**
* A class of callables that are potentially relevant for generating sink models.
*/
class SinkTargetApi extends TargetApiBase {
SinkTargetApi() { not hasManualSinkModel(this.lift()) }
class SinkTargetApi extends SourceOrSinkTargetApi {
SinkTargetApi() { not hasManualSinkModel(this) }
}

/**
* A class of callables that are potentially relevant for generating source models.
*/
class SourceTargetApi extends TargetApiBase {
SourceTargetApi() { not hasManualSourceModel(this.lift()) }
class SourceTargetApi extends SourceOrSinkTargetApi {
SourceTargetApi() { not hasManualSourceModel(this) }
}

/**
Expand All @@ -112,16 +112,19 @@ class SourceTargetApi extends TargetApiBase {
predicate isUninterestingForTypeBasedFlowModels(Callable api) { none() }

/**
* A class of callables that are potentially relevant for generating summary, source, sink
* and neutral models.
* A class of callables that are potentially relevant for generating summary or
* neutral models.
*
* In the Standard library and 3rd party libraries it is the callables (or callables that have a
* super implementation) that can be called from outside the library itself.
*/
class TargetApiBase extends Callable {
class SummaryTargetApi extends Callable {
private Callable lift;

TargetApiBase() { lift = liftedImpl(this) }
SummaryTargetApi() {
lift = liftedImpl(this) and
not hasManualSummaryModel(lift)
}

/**
* Gets the callable that a model will be lifted to.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@ private predicate output(Callable callable, TypeVariable tv, string output) {
}

module ModelPrintingInput implements ModelPrintingSig {
class Api = TypeBasedFlowTargetApi;
class SummaryApi = TypeBasedFlowTargetApi;

class SourceOrSinkApi = Specific::SourceOrSinkTargetApi;

string getProvenance() { result = "tb-generated" }
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ extensions:
extensible: sinkModel
data:
- [ "p", "Sinks", False, "sink", "(Object)", "", "Argument[0]", "test-sink", "manual" ]
- [ "p", "Sinks", False, "sink2", "(Object)", "", "Argument[0]", "test-sink2", "manual" ]
- [ "p", "Sinks", False, "manualSinkAlreadyDefined", "(Object)", "", "Argument[0]", "test-sink", "manual" ]

- addsTo:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ extensions:
extensible: sourceModel
data:
- [ "p", "Sources", False, "source", "()", "", "ReturnValue", "test-source", "manual" ]
- [ "p", "Sources", False, "source2", "()", "", "ReturnValue", "test-source2", "manual" ]
- [ "p", "Sources", False, "manualSourceAlreadyDefined", "()", "", "ReturnValue", "test-source", "manual" ]

- addsTo:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

public class ImplOfExternalSPI extends AbstractImplOfExternalSPI {

// sink=p;AbstractImplOfExternalSPI;true;accept;(File);;Argument[0];path-injection;df-generated
// sink=p;ImplOfExternalSPI;true;accept;(File);;Argument[0];path-injection;df-generated
// neutral=p;ImplOfExternalSPI;accept;(File);summary;df-generated
@Override
public boolean accept(File pathname) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public PrivateImplWithSink(File file) {
}

// summary=p;PrivateFlowViaPublicInterface$SPI;true;openStream;();;Argument[this];ReturnValue;taint;df-generated
// sink=p;PrivateFlowViaPublicInterface$SPI;true;openStream;();;Argument[this];path-injection;df-generated
@Override
public OutputStream openStream() throws IOException {
return new FileOutputStream(file);
Expand Down
27 changes: 27 additions & 0 deletions java/ql/test/utils/modelgenerator/dataflow/p/Sinks.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ public class Sinks {
// neutral=p;Sinks;sink;(Object);summary;df-generated
public void sink(Object o) {}

// Defined as a sink in the model file next to the test.
// neutral=p;Sinks;sink2;(Object);summary;df-generated
public void sink2(Object o) {}

// sink=p;Sinks;true;copyFileToDirectory;(Path,Path,CopyOption[]);;Argument[0];path-injection;df-generated
// sink=p;Sinks;true;copyFileToDirectory;(Path,Path,CopyOption[]);;Argument[1];path-injection;df-generated
// neutral=p;Sinks;copyFileToDirectory;(Path,Path,CopyOption[]);summary;df-generated
Expand Down Expand Up @@ -77,4 +81,27 @@ public void wrapSinkSimpleType(String s) {
public void manualSinkAlreadyDefined(Object o) {
sink(o);
}

public abstract class DataWriter {
// neutral=p;Sinks$DataWriter;write;(String);summary;df-generated
public abstract void write(String s);
}

public class DataWriterKind1 extends DataWriter {
// sink=p;Sinks$DataWriterKind1;true;write;(String);;Argument[0];test-sink;df-generated
// neutral=p;Sinks$DataWriterKind1;write;(String);summary;df-generated
@Override
public void write(String s) {
sink(s);
}
}

public class DataWriterKind2 extends DataWriter {
// sink=p;Sinks$DataWriterKind2;true;write;(String);;Argument[0];test-sink2;df-generated
// neutral=p;Sinks$DataWriterKind2;write;(String);summary;df-generated
@Override
public void write(String s) {
sink2(s);
}
}
}
29 changes: 29 additions & 0 deletions java/ql/test/utils/modelgenerator/dataflow/p/Sources.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,12 @@ public String source() {
return "";
}

// Defined as a source in the model file next to the test.
// neutral=p;Sources;source2;();summary;df-generated
public String source2() {
return "";
}

// source=p;Sources;true;readUrl;(URL);;ReturnValue;remote;df-generated
// sink=p;Sources;true;readUrl;(URL);;Argument[0];request-forgery;df-generated
// neutral=p;Sources;readUrl;(URL);summary;df-generated
Expand Down Expand Up @@ -79,4 +85,27 @@ public String manualNeutralSource() {
public String manualSourceAlreadyDefined() {
return source();
}

public abstract class DataReader {
// neutral=p;Sources$DataReader;read;();summary;df-generated
public abstract String read();
}

public class DataReaderKind1 extends DataReader {
// source=p;Sources$DataReaderKind1;true;read;();;ReturnValue;test-source;df-generated
// neutral=p;Sources$DataReaderKind1;read;();summary;df-generated
@Override
public String read() {
return source();
}
}

public class DataReaderKind2 extends DataReader {
// source=p;Sources$DataReaderKind2;true;read;();;ReturnValue;test-source2;df-generated
// neutral=p;Sources$DataReaderKind2;read;();summary;df-generated
@Override
public String read() {
return source2();
}
}
}
Loading

0 comments on commit 25b2018

Please sign in to comment.