Skip to content

Commit 1d1c476

Browse files
am0o0am0o0
committed
update tests and use TaintFlowTestArgString
add stubs add missed sink models
1 parent 1cb9f63 commit 1d1c476

File tree

590 files changed

+19282
-142
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

590 files changed

+19282
-142
lines changed

‎java/ql/lib/ext/experimental/java.nio.model.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sinkModel
5+
data:
6+
- ["java.nio.file","FileSystems",true,"getFileSystem","(URI)","","Argument[0]","path-injection","manual"]
7+
- ["java.nio.channels","AsynchronousFileChannel",true,"open","(Path,OpenOption[])","","Argument[0]","path-injection","manual"]
8+
- ["java.nio.channels","AsynchronousFileChannel",true,"open","(Path,Set,ExecutorService,FileAttribute[])","","Argument[0]","path-injection","manual"]

‎java/ql/lib/ext/experimental/java.util.zip.model.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
extensions:
2+
- addsTo:
3+
pack: codeql/java-all
4+
extensible: sinkModel
5+
data:
6+
- ["java.util.zip","ZipFile",true,"ZipFile","(String)","","Argument[0]","path-injection","manual"]

‎java/ql/lib/ext/experimental/s3-transfer-manager.model.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,4 @@ extensions:
1010
- ["software.amazon.awssdk.transfer.s3.model","ResumableFileDownload",true,"fromFile","(Path)","","Argument[0]","path-injection","manual"]
1111
- ["software.amazon.awssdk.transfer.s3.model","ResumableFileDownload",true,"serializeToFile","(Path)","","Argument[0]","path-injection","manual"]
1212
- ["software.amazon.awssdk.transfer.s3.model","ResumableFileUpload",true,"fromFile","(Path)","","Argument[0]","path-injection","manual"]
13-
- ["software.amazon.awssdk.transfer.s3.model","UploadDirectoryRequest$Builder",true,"source","(Path)","","Argument[0]","code-injection","manual"]
13+
- ["software.amazon.awssdk.transfer.s3.model","UploadDirectoryRequest$Builder",true,"source","(Path)","","Argument[0]","path-injection","manual"]

‎java/ql/src/utils/modeleditor/FrameworkModeEndpointsQuery.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
private import java
2+
private import semmle.code.java.dataflow.ExternalFlow
23
private import semmle.code.java.dataflow.internal.DataFlowPrivate
3-
private import semmle.code.java.dataflow.internal.FlowSummaryImpl
44
private import semmle.code.java.dataflow.internal.ModelExclusions
55
private import ModelEditor
66

77
/**
88
* A class of effectively public callables from source code.
99
*/
1010
class PublicEndpointFromSource extends Endpoint, ModelApi {
11-
override predicate isSource() { SourceSinkInterpretationInput::sourceElement(this, _, _, _, _) }
11+
override predicate isSource() { this instanceof SourceCallable }
1212

13-
override predicate isSink() { SourceSinkInterpretationInput::sinkElement(this, _, _, _, _) }
13+
override predicate isSink() { this instanceof SinkCallable }
1414
}

‎java/ql/src/utils/modelgenerator/internal/CaptureModels.qll

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,18 +41,21 @@ private module Printing implements PrintingSig {
4141

4242
module ModelPrinting = PrintingImpl<Printing>;
4343

44+
/**
45+
* Holds if `c` is a relevant content kind, where the underlying type is relevant.
46+
*/
47+
private predicate isRelevantTypeInContent(DataFlow::Content c) {
48+
isRelevantType(getUnderlyingContentType(c))
49+
}
50+
4451
/**
4552
* Holds if data can flow from `node1` to `node2` either via a read or a write of an intermediate field `f`.
4653
*/
4754
private predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
4855
exists(DataFlow::Content f |
4956
DataFlowPrivate::readStep(node1, f, node2) and
50-
if f instanceof DataFlow::FieldContent
51-
then isRelevantType(f.(DataFlow::FieldContent).getField().getType())
52-
else
53-
if f instanceof DataFlow::SyntheticFieldContent
54-
then isRelevantType(f.(DataFlow::SyntheticFieldContent).getField().getType())
55-
else any()
57+
// Partially restrict the content types used for intermediate steps.
58+
(not exists(getUnderlyingContentType(f)) or isRelevantTypeInContent(f))
5659
)
5760
or
5861
exists(DataFlow::Content f | DataFlowPrivate::storeStep(node1, f, node2) |
@@ -61,12 +64,11 @@ private predicate isRelevantTaintStep(DataFlow::Node node1, DataFlow::Node node2
6164
}
6265

6366
/**
64-
* Holds if content `c` is either a field or synthetic field of a relevant type
65-
* or a container like content.
67+
* Holds if content `c` is either a field, a synthetic field or language specific
68+
* content of a relevant type or a container like content.
6669
*/
6770
private predicate isRelevantContent(DataFlow::Content c) {
68-
isRelevantType(c.(DataFlow::FieldContent).getField().getType()) or
69-
isRelevantType(c.(DataFlow::SyntheticFieldContent).getField().getType()) or
71+
isRelevantTypeInContent(c) or
7072
DataFlowPrivate::containerContent(c)
7173
}
7274

@@ -258,6 +260,10 @@ module PropagateToSinkConfig implements DataFlow::ConfigSig {
258260
predicate isBarrier(DataFlow::Node node) { sinkModelSanitizer(node) }
259261

260262
DataFlow::FlowFeature getAFeature() { result instanceof DataFlow::FeatureHasSourceCallContext }
263+
264+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
265+
isRelevantTaintStep(node1, node2)
266+
}
261267
}
262268

263269
private module PropagateToSink = TaintTracking::Global<PropagateToSinkConfig>;

‎java/ql/src/utils/modelgenerator/internal/CaptureModelsSpecific.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,14 @@ predicate isRelevantType(J::Type t) {
186186
)
187187
}
188188

189+
/**
190+
* Gets the underlying type of the content `c`.
191+
*/
192+
J::Type getUnderlyingContentType(DataFlow::Content c) {
193+
result = c.(DataFlow::FieldContent).getField().getType() or
194+
result = c.(DataFlow::SyntheticFieldContent).getField().getType()
195+
}
196+
189197
/**
190198
* Gets the MaD string representation of the qualifier.
191199
*/

‎java/ql/test/experimental/query-tests/security/CWE-022/src/main/java/com/PathInjection/S3PathInjection.java renamed to ‎java/ql/test/experimental/query-tests/security/CWE-022/AmazonS3.java

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
1-
package com.PathInjection;
2-
31
import software.amazon.awssdk.transfer.s3.S3TransferManager;
4-
import software.amazon.awssdk.transfer.s3.model.*;
2+
import software.amazon.awssdk.transfer.s3.model.UploadFileRequest;
3+
import software.amazon.awssdk.transfer.s3.model.FileUpload;
4+
import software.amazon.awssdk.transfer.s3.model.FileDownload;
5+
import software.amazon.awssdk.transfer.s3.model.DirectoryUpload;
6+
import software.amazon.awssdk.transfer.s3.model.CompletedDirectoryUpload;
7+
import software.amazon.awssdk.transfer.s3.model.DirectoryDownload;
8+
import software.amazon.awssdk.transfer.s3.model.CompletedDirectoryDownload;
9+
import software.amazon.awssdk.transfer.s3.model.DownloadDirectoryRequest;
10+
import software.amazon.awssdk.transfer.s3.model.DownloadFileRequest;
11+
import software.amazon.awssdk.transfer.s3.model.ResumableFileUpload;
12+
import software.amazon.awssdk.transfer.s3.model.UploadDirectoryRequest;
13+
import software.amazon.awssdk.transfer.s3.model.ResumableFileDownload;
14+
import software.amazon.awssdk.transfer.s3.model.CompletedFileUpload;
15+
import software.amazon.awssdk.transfer.s3.model.CompletedFileDownload;
516
import software.amazon.awssdk.transfer.s3.progress.LoggingTransferListener;
617

7-
import java.net.MalformedURLException;
818
import java.net.URI;
9-
import java.net.URISyntaxException;
1019
import java.nio.file.Paths;
1120

12-
public class S3PathInjection {
21+
public class AmazonS3 {
1322
S3TransferManager transferManager = S3TransferManager.create();
1423
String bucketName = "bucketTest";
1524
String key = "keyTest";
@@ -19,7 +28,7 @@ public String uploadFile(URI filePathURI) {
1928
UploadFileRequest.builder()
2029
.putObjectRequest(b -> b.bucket(this.bucketName).key(this.key))
2130
.addTransferListener(LoggingTransferListener.create())
22-
.source(Paths.get(filePathURI)) // $ hasTaintFlow
31+
.source(Paths.get(filePathURI)) // $ hasTaintFlow="get(...)"
2332
.build();
2433

2534
FileUpload fileUpload = this.transferManager.uploadFile(uploadFileRequest);
@@ -33,18 +42,18 @@ public String uploadFileResumable(URI filePathURI) {
3342
UploadFileRequest.builder()
3443
.putObjectRequest(b -> b.bucket(this.bucketName).key(this.key))
3544
.addTransferListener(LoggingTransferListener.create())
36-
.source(Paths.get(filePathURI)) // $ hasTaintFlow
45+
.source(Paths.get(filePathURI)) // $ hasTaintFlow="get(...)"
3746
.build();
3847

3948
// Initiate the transfer
4049
FileUpload upload = this.transferManager.uploadFile(uploadFileRequest);
4150
// Pause the upload
4251
ResumableFileUpload resumableFileUpload = upload.pause();
4352
// Optionally, persist the resumableFileUpload
44-
resumableFileUpload.serializeToFile(Paths.get(filePathURI)); // $ hasTaintFlow
53+
resumableFileUpload.serializeToFile(Paths.get(filePathURI)); // $ hasTaintFlow="get(...)"
4554
// Retrieve the resumableFileUpload from the file
4655
ResumableFileUpload persistedResumableFileUpload =
47-
ResumableFileUpload.fromFile(Paths.get(filePathURI)); // $ hasTaintFlow
56+
ResumableFileUpload.fromFile(Paths.get(filePathURI)); // $ hasTaintFlow="get(...)"
4857
// Resume the upload
4958
FileUpload resumedUpload = this.transferManager.resumeUploadFile(persistedResumableFileUpload);
5059
// Wait for the transfer to complete
@@ -59,18 +68,18 @@ public String downloadFileResumable(URI downloadedFileWithPath) {
5968
DownloadFileRequest.builder()
6069
.getObjectRequest(b -> b.bucket(this.bucketName).key(this.key))
6170
.addTransferListener(LoggingTransferListener.create())
62-
.destination(Paths.get(downloadedFileWithPath)) // $ hasTaintFlow
71+
.destination(Paths.get(downloadedFileWithPath)) // $ hasTaintFlow="get(...)"
6372
.build();
6473

6574
// Initiate the transfer
6675
FileDownload download = this.transferManager.downloadFile(downloadFileRequest);
6776
// Pause the download
6877
ResumableFileDownload resumableFileDownload = download.pause();
6978
// Optionally, persist the resumableFileDownload
70-
resumableFileDownload.serializeToFile(Paths.get(downloadedFileWithPath)); // $ hasTaintFlow
79+
resumableFileDownload.serializeToFile(Paths.get(downloadedFileWithPath)); // $ hasTaintFlow="get(...)"
7180
// Retrieve the resumableFileDownload from the file
7281
ResumableFileDownload persistedResumableFileDownload =
73-
ResumableFileDownload.fromFile(Paths.get(downloadedFileWithPath)); // $ hasTaintFlow
82+
ResumableFileDownload.fromFile(Paths.get(downloadedFileWithPath)); // $ hasTaintFlow="get(...)"
7483
// Resume the download
7584
FileDownload resumedDownload =
7685
this.transferManager.resumeDownloadFile(persistedResumableFileDownload);
@@ -85,7 +94,7 @@ public Integer uploadDirectory(URI sourceDirectory) {
8594
DirectoryUpload directoryUpload =
8695
this.transferManager.uploadDirectory(
8796
UploadDirectoryRequest.builder()
88-
.source(Paths.get(sourceDirectory)) // $ hasTaintFlow
97+
.source(Paths.get(sourceDirectory)) // $ hasTaintFlow="get(...)"
8998
.bucket(this.bucketName)
9099
.build());
91100

@@ -98,7 +107,7 @@ public Long downloadFile(String downloadedFileWithPath) {
98107
DownloadFileRequest.builder()
99108
.getObjectRequest(b -> b.bucket(this.bucketName).key(this.key))
100109
.addTransferListener(LoggingTransferListener.create())
101-
.destination(Paths.get(downloadedFileWithPath)) // $ hasTaintFlow
110+
.destination(Paths.get(downloadedFileWithPath)) // $ hasTaintFlow="get(...)"
102111
.build();
103112

104113
FileDownload downloadFile = this.transferManager.downloadFile(downloadFileRequest);
@@ -111,7 +120,7 @@ public Integer downloadObjectsToDirectory(URI destinationPathURI) {
111120
DirectoryDownload directoryDownload =
112121
this.transferManager.downloadDirectory(
113122
DownloadDirectoryRequest.builder()
114-
.destination(Paths.get(destinationPathURI)) // $ hasTaintFlow
123+
.destination(Paths.get(destinationPathURI)) // $ hasTaintFlow="get(...)"
115124
.bucket(this.bucketName)
116125
.build());
117126
CompletedDirectoryDownload completedDirectoryDownload =

‎java/ql/test/experimental/query-tests/security/CWE-022/src/main/java/com/PathInjection/CommonsIOPathInjection.java renamed to ‎java/ql/test/experimental/query-tests/security/CWE-022/JavaNio.java

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,39 @@
1-
package com.PathInjection;
2-
3-
import java.io.*;
1+
import java.io.IOException;
2+
import java.io.File;
43
import java.nio.channels.AsynchronousFileChannel;
5-
import java.nio.file.*;
4+
import java.nio.file.Path;
5+
import java.nio.file.LinkOption;
6+
import java.nio.file.FileSystems;
67
import java.nio.file.attribute.FileAttribute;
78
import java.util.Set;
89
import java.util.concurrent.ExecutorService;
910
import java.util.concurrent.Executors;
1011

11-
class fileAttr implements FileAttribute<String> {
12-
public String name() {
13-
return "file";
14-
}
12+
public class JavaNio {
13+
static class FileAttr implements FileAttribute<String> {
14+
public String name() {
15+
return "file";
16+
}
1517

16-
public String value() {
17-
return "value";
18+
public String value() {
19+
return "value";
20+
}
1821
}
19-
}
2022

21-
public class CommonsIOPathInjection {
2223
public void PathInjection(Path src, File srcF) throws IOException {
23-
AsynchronousFileChannel.open(src); // $ hasTaintFlow
24-
AsynchronousFileChannel.open(src, LinkOption.NOFOLLOW_LINKS); // $ hasTaintFlow
24+
AsynchronousFileChannel.open(src); // $ hasTaintFlow="src"
25+
AsynchronousFileChannel.open(src, LinkOption.NOFOLLOW_LINKS); // $ hasTaintFlow="src"
2526
AsynchronousFileChannel.open(
26-
src, LinkOption.NOFOLLOW_LINKS, LinkOption.NOFOLLOW_LINKS); // $ hasTaintFlow
27+
src, LinkOption.NOFOLLOW_LINKS, LinkOption.NOFOLLOW_LINKS); // $ hasTaintFlow="src"
2728
ExecutorService executor = Executors.newFixedThreadPool(10);
2829
AsynchronousFileChannel.open(
29-
src, Set.of(LinkOption.NOFOLLOW_LINKS), executor); // $ hasTaintFlow
30+
src, Set.of(LinkOption.NOFOLLOW_LINKS), executor); // $ hasTaintFlow="src"
3031
AsynchronousFileChannel.open(
31-
src, // $ hasTaintFlow
32+
src, // $ hasTaintFlow="src"
3233
Set.of(LinkOption.NOFOLLOW_LINKS),
3334
executor,
34-
new fileAttr());
35+
new FileAttr());
3536

36-
FileSystems.getFileSystem(srcF.toURI()); // $ hasTaintFlow
37+
FileSystems.getFileSystem(srcF.toURI()); // $ hasTaintFlow="toURI(...)"
3738
}
3839
}

‎java/ql/test/experimental/query-tests/security/CWE-022/Main.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
import java.io.BufferedReader;
2+
import java.io.InputStreamReader;
3+
import java.io.File;
4+
import java.io.IOException;
5+
import java.nio.charset.StandardCharsets;
6+
import java.nio.file.Path;
7+
import java.net.Socket;
8+
9+
public class Main {
10+
public void sendUserFileGood(Socket sock) throws IOException {
11+
BufferedReader filenameReader =
12+
new BufferedReader(new InputStreamReader(sock.getInputStream(), StandardCharsets.UTF_8));
13+
String path = filenameReader.readLine();
14+
Path src = Path.of(path);
15+
File srcF = new File(path);
16+
17+
new JavaNio().PathInjection(src, srcF);
18+
19+
new SpringIo().PathInjection(path);
20+
21+
AmazonS3 s3PathInjection = new AmazonS3();
22+
s3PathInjection.downloadFileResumable(src.toUri());
23+
s3PathInjection.downloadFile(path);
24+
s3PathInjection.downloadObjectsToDirectory(src.toUri());
25+
s3PathInjection.uploadFileResumable(src.toUri());
26+
s3PathInjection.uploadDirectory(src.toUri());
27+
s3PathInjection.uploadFile(src.toUri());
28+
29+
Zip4j zip4jfile = new Zip4j();
30+
zip4jfile.PathInjection(path);
31+
32+
ZipFile zipfile = new ZipFile();
33+
zipfile.PathInjection(path);
34+
}
35+
}

‎java/ql/test/experimental/query-tests/security/CWE-022/PathInjection.iml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<module version="4">
3+
<component name="AdditionalModuleElements">
4+
<content url="file://$MODULE_DIR$" dumb="true">
5+
<sourceFolder url="file://$MODULE_DIR$" isTestSource="false" />
6+
</content>
7+
</component>
8+
</module>

0 commit comments

Comments
 (0)