Apply suggestions from code review

Co-authored-by: Chris Smowton <smowton@github.com>
This commit is contained in:
Jonathan Leitschuh
2021-04-19 11:54:35 -04:00
committed by Jonathan Leitschuh
parent a8d25b63ac
commit a4b5573f53
5 changed files with 15 additions and 8 deletions

View File

@@ -19,10 +19,10 @@ can occur.</p>
<recommendation>
<p>Use JDK methods that specifically protect against this vulnerability:</p>
<ul>
<li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempDirectory-java.nio.file.Path-java.lang.String-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files#createTempDirectory</a></li>
<li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempDirectory-java.nio.file.Path-java.lang.String-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files.createTempDirectory</a></li>
<li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempFile-java.nio.file.Path-java.lang.String-java.lang.String-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files#createTempFile</a></li>
</ul>
<p>Otherwise, create the file/directory by manually specificfying the expected posix file permissions.
<p>Otherwise, create the file/directory by manually specifying the expected posix file permissions.
For example: <code>PosixFilePermissions.asFileAttribute(EnumSet.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE))</code></p>
<ul>
<li><a href="https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createFile-java.nio.file.Path-java.nio.file.attribute.FileAttribute...-">java.nio.file.Files#createFile</a></li>

View File

@@ -38,6 +38,9 @@ class MethodAccessInsecureFileCreateTempFile extends MethodAccessInsecureFileCre
override string getFileSystemType() { result = "file" }
}
/**
* The `com.google.common.io.Files.createTempDir` method.
*/
class MethodGuavaFilesCreateTempFile extends Method {
MethodGuavaFilesCreateTempFile() {
getDeclaringType().hasQualifiedName("com.google.common.io", "Files") and
@@ -45,6 +48,9 @@ class MethodGuavaFilesCreateTempFile extends Method {
}
}
/**
* A call to the `com.google.common.io.Files.createTempDir` method.
*/
class MethodAccessInsecureGuavaFilesCreateTempFile extends MethodAccessInsecureFileCreation {
MethodAccessInsecureGuavaFilesCreateTempFile() {
getMethod() instanceof MethodGuavaFilesCreateTempFile

View File

@@ -24,7 +24,7 @@ private class MethodFileSystemFileCreation extends Method {
abstract private class FileCreationSink extends DataFlow::Node { }
/**
* Sink for tainted `File` having a file or directory creation method called on it.
* The qualifier of a call to one of `File`'s file-creating or directory-creating methods, treated as a sink by `TempDirSystemGetPropertyToCreateConfig`.
*/
private class FileFileCreationSink extends FileCreationSink {
FileFileCreationSink() {
@@ -36,7 +36,8 @@ private class FileFileCreationSink extends FileCreationSink {
}
/**
* Sink for calling a file-creating or directory-creating `Files` method on a tainted `File` or `Path`.
* The argument to
a call to one of `Files` file-creating or directory-creating methods, treated as a sink by `TempDirSystemGetPropertyToCreateConfig`.
*/
private class FilesFileCreationSink extends FileCreationSink {
FilesFileCreationSink() {
@@ -45,8 +46,8 @@ private class FilesFileCreationSink extends FileCreationSink {
}
/**
* Captures all of the vulnerable methods on `Files` that create files/directories without explicitly
* setting the permissions.
* A call to a `Files` method that create files/directories without explicitly
* setting the newly-created file or directory's permissions.
*/
private class FilesVulnerableCreationMethodAccess extends MethodAccess {
FilesVulnerableCreationMethodAccess() {

View File

@@ -49,7 +49,7 @@ class MethodFileCreateTempFile extends Method {
* - `new File(System.getProperty("java.io.tmpdir"))`
* - `new File(new File(System.getProperty("java.io.tmpdir")), "/child")`
*/
private predicate isTaintedFileCreation(Expr expSource, Expr exprDest) {
private predicate isFileConstructorArgument(Expr expSource, Expr exprDest) {
exists(ConstructorCall construtorCall |
construtorCall.getConstructedType() instanceof TypeFile and
construtorCall.getArgument(0) = expSource and