From 086c473c18d1d1ef096253d371ff680b92ea45ce Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Wed, 11 Sep 2019 12:05:33 +0200 Subject: [PATCH] JS: sharpen js/http-to-file-access --- change-notes/1.23/analysis-javascript.md | 1 + .../ql/src/Security/CWE-912/HttpToFileAccess.ql | 4 ++-- .../dataflow/HttpToFileAccessCustomizations.qll | 14 +++++++++++++- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/change-notes/1.23/analysis-javascript.md b/change-notes/1.23/analysis-javascript.md index 75c47f50d68..fd13c44c16a 100644 --- a/change-notes/1.23/analysis-javascript.md +++ b/change-notes/1.23/analysis-javascript.md @@ -22,6 +22,7 @@ | Client-side cross-site scripting (`js/xss`) | More results | More potential vulnerabilities involving functions that manipulate DOM attributes are now recognized. | | Code injection (`js/code-injection`) | More results | More potential vulnerabilities involving functions that manipulate DOM event handler attributes are now recognized. | | Incorrect suffix check (`js/incorrect-suffix-check`) | Fewer false-positive results | The query recognizes valid checks in more cases. +| Network data written to file (`js/http-to-file-access`) | Fewer false-positive results | This query has been renamed to better match its intended purpose, and now only considers network data untrusted. | | Prototype pollution (`js/prototype-pollution`) | More results | The query now highlights vulnerable uses of jQuery and Angular, and the results are shown on LGTM by default. | | Uncontrolled command line (`js/command-line-injection`) | More results | This query now treats responses from servers as untrusted. | diff --git a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql b/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql index c592eaf3e30..b3b8d52a869 100644 --- a/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql +++ b/javascript/ql/src/Security/CWE-912/HttpToFileAccess.ql @@ -1,6 +1,6 @@ /** - * @name User-controlled data written to file - * @description Writing user-controlled data directly to the file system allows arbitrary file upload and might indicate a backdoor. + * @name Network data written to file + * @description Writing network data directly to the file system allows arbitrary file upload and might indicate a backdoor. * @kind path-problem * @problem.severity warning * @precision medium diff --git a/javascript/ql/src/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll b/javascript/ql/src/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll index 4fbe0349fcf..f2de8f5fcc1 100644 --- a/javascript/ql/src/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll +++ b/javascript/ql/src/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll @@ -24,10 +24,22 @@ module HttpToFileAccess { abstract class Sanitizer extends DataFlow::Node { } /** A source of remote user input, considered as a flow source for writing user-controlled data to files. */ - class RemoteFlowSourceAsSource extends Source { + deprecated class RemoteFlowSourceAsSource extends DataFlow::Node { RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource } } + /** + * An access to a user-controlled HTTP request input, considered as a flow source for writing user-controlled data to files + */ + private class RequestInputAccessAsSource extends Source { + RequestInputAccessAsSource() { this instanceof HTTP::RequestInputAccess } + } + + /** A response from a server, considered as a flow source for writing user-controlled data to files. */ + private class ServerResponseAsSource extends Source { + ServerResponseAsSource() { this = any(ClientRequest r).getAResponseDataNode() } + } + /** A sink that represents file access method (write, append) argument */ class FileAccessAsSink extends Sink { FileAccessAsSink() { exists(FileSystemWriteAccess src | this = src.getADataNode()) }