mirror of
https://github.com/github/codeql.git
synced 2025-12-16 16:53:25 +01:00
Go: Promote go/uncontrolled-allocation-size
This commit is contained in:
Tony Torralba
34
go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll
Normal file
34
go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll
Normal file
@@ -0,0 +1,34 @@
|
||||
/**
|
||||
* Provides a taint-tracking configuration for reasoning about uncontrolled allocation size issues.
|
||||
*/
|
||||
|
||||
import go
|
||||
|
||||
/**
|
||||
* Provides a taint-tracking flow for reasoning about uncontrolled allocation size issues.
|
||||
*/
|
||||
module UncontrolledAllocationSize {
|
||||
private import UncontrolledAllocationSizeCustomizations::UncontrolledAllocationSize
|
||||
|
||||
/**
|
||||
* Module for defining predicates and tracking taint flow related to uncontrolled allocation size issues.
|
||||
*/
|
||||
module Config implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) { source instanceof Source }
|
||||
|
||||
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
|
||||
|
||||
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(Function f, DataFlow::CallNode cn | cn = f.getACall() |
|
||||
f.hasQualifiedName("strconv", ["Atoi", "ParseInt", "ParseUint", "ParseFloat"]) and
|
||||
node1 = cn.getArgument(0) and
|
||||
node2 = cn.getResult(0)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/** Tracks taint flow for reasoning about uncontrolled allocation size issues. */
|
||||
module Flow = TaintTracking::Global<Config>;
|
||||
}
|
||||
@@ -0,0 +1,33 @@
|
||||
/**
|
||||
* Provides default sources, sinks, and sanitizers for reasoning about uncontrolled allocation size issues,
|
||||
* as well as extension points for adding your own.
|
||||
*/
|
||||
|
||||
import go
|
||||
private import semmle.go.security.AllocationSizeOverflow
|
||||
|
||||
/**
|
||||
* Provides extension points for customizing the taint-tracking configuration for reasoning
|
||||
* about uncontrolled allocation size issues.
|
||||
*/
|
||||
module UncontrolledAllocationSize {
|
||||
/** A data flow source for uncontrolled allocation size vulnerabilities. */
|
||||
abstract class Source extends DataFlow::Node { }
|
||||
|
||||
/** A data flow sink for uncontrolled allocation size vulnerabilities. */
|
||||
abstract class Sink extends DataFlow::Node { }
|
||||
|
||||
/** A sanitizer for uncontrolled allocation size vulnerabilities. */
|
||||
abstract class Sanitizer extends DataFlow::Node { }
|
||||
|
||||
/** A source of untrusted data, considered as a taint source for uncontrolled size allocation vulnerabilities. */
|
||||
private class UntrustedFlowAsSource extends Source instanceof UntrustedFlowSource { }
|
||||
|
||||
/** The size argument of a memory allocation function. */
|
||||
private class AllocationSizeAsSink extends Sink instanceof AllocationSizeOverflow::AllocationSize {
|
||||
}
|
||||
|
||||
/** A check that a value is below some upper limit. */
|
||||
private class SizeCheckSanitizer extends Sanitizer instanceof AllocationSizeOverflow::AllocationSizeCheckBarrier
|
||||
{ }
|
||||
}
|
||||
36
go/ql/src/Security/CWE-770/UncontrolledAllocationSize.qhelp
Normal file
36
go/ql/src/Security/CWE-770/UncontrolledAllocationSize.qhelp
Normal file
@@ -0,0 +1,36 @@
|
||||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
|
||||
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>Using untrusted input to allocate slices with the built-in <code>make</code> function could
|
||||
lead to excessive memory allocation and potentially cause the program to crash due to running
|
||||
out of memory. This vulnerability could be exploited to perform a denial-of-service attack by
|
||||
consuming all available server resources.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Implement a maximum allowed value for size allocations with the built-in <code>make</code>
|
||||
function to prevent excessively large allocations.</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>In the following example snippet, the <code>n</code> parameter is user-controlled.</p>
|
||||
<p>If the external user provides an excessively large value, the application allocates a slice
|
||||
of size <code>n</code> without further verification, potentially exhausting all the available
|
||||
memory.</p>
|
||||
|
||||
<sample src="UncontrolledAllocationSize.go" />
|
||||
|
||||
<p>One way to prevent this vulnerability is by implementing a maximum allowed value for the
|
||||
user-controlled input, as seen in the following example:</p>
|
||||
|
||||
<sample src="UncontrolledAllocationSize.go" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li> OWASP: <a
|
||||
href="https://cheatsheetseries.owasp.org/cheatsheets/Denial_of_Service_Cheat_Sheet.html">Denial
|
||||
of Service Cheat Sheet</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
22
go/ql/src/Security/CWE-770/UncontrolledAllocationSize.ql
Normal file
22
go/ql/src/Security/CWE-770/UncontrolledAllocationSize.ql
Normal file
@@ -0,0 +1,22 @@
|
||||
/**
|
||||
* @name Slice memory allocation with excessive size value
|
||||
* @description Allocating memory for slices with the built-in make function from user-controlled sources
|
||||
* can lead to a denial of service.
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 6.0
|
||||
* @precision high
|
||||
* @id go/uncontrolled-allocation-size
|
||||
* @tags security
|
||||
* external/cwe/cwe-770
|
||||
*/
|
||||
|
||||
import go
|
||||
import semmle.go.security.UncontrolledAllocationSize
|
||||
import UncontrolledAllocationSize::Flow::PathGraph
|
||||
|
||||
from
|
||||
UncontrolledAllocationSize::Flow::PathNode source, UncontrolledAllocationSize::Flow::PathNode sink
|
||||
where UncontrolledAllocationSize::Flow::flowPath(source, sink)
|
||||
select sink, source, sink, "This memory allocation depends on a $@.", source.getNode(),
|
||||
"user-provided value"
|
||||
@@ -0,0 +1,4 @@
|
||||
---
|
||||
category: newQuery
|
||||
---
|
||||
* The query "Slice memory allocation with excessive size value" (`go/uncontrolled-allocation-size`) has been promoted from experimental to the main query pack. Its results will now appear by default. This query was originally [submitted as an experimental query by @Malayke](https://github.com/github/codeql/pull/15130).
|
||||
@@ -1,32 +0,0 @@
|
||||
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
|
||||
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>Using untrusted input to created with the built-in make function
|
||||
could lead to excessive memory allocation and potentially cause the program to crash due
|
||||
to running out of memory. This vulnerability could be exploited to perform a DoS attack by consuming all available server resources.</p>
|
||||
</overview>
|
||||
|
||||
<recommendation>
|
||||
<p>Implement a maximum allowed value for creates a slice with the built-in make function to prevent excessively large allocations.
|
||||
For instance, you could restrict it to a reasonable upper limit.</p>
|
||||
</recommendation>
|
||||
|
||||
<example>
|
||||
<p>In the following example snippet, the <code>n</code> field is user-controlled.</p>
|
||||
<p> The server trusts that n has an acceptable value, however when using a maliciously large value,
|
||||
it allocates a slice of <code>n</code> of strings before filling the slice with data.</p>
|
||||
|
||||
<sample src="DenialOfServiceBad.go" />
|
||||
|
||||
<p>One way to prevent this vulnerability is by implementing a maximum allowed value for the user-controlled input:</p>
|
||||
|
||||
<sample src="DenialOfServiceGood.go" />
|
||||
</example>
|
||||
|
||||
<references>
|
||||
<li>
|
||||
OWASP: <a href="https://cheatsheetseries.owasp.org/cheatsheets/Denial_of_Service_Cheat_Sheet.html">Denial of Service Cheat Sheet</a>
|
||||
</li>
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -1,59 +0,0 @@
|
||||
/**
|
||||
* @name Denial Of Service
|
||||
* @description slices created with the built-in make function from user-controlled sources using a
|
||||
* maliciously large value possibly leading to a denial of service.
|
||||
* @kind path-problem
|
||||
* @problem.severity error
|
||||
* @security-severity 9
|
||||
* @precision high
|
||||
* @id go/denial-of-service
|
||||
* @tags security
|
||||
* experimental
|
||||
* external/cwe/cwe-770
|
||||
*/
|
||||
|
||||
import go
|
||||
|
||||
/**
|
||||
* Holds if the guard `g` on its branch `branch` checks that `e` is not constant and is less than some other value.
|
||||
*/
|
||||
predicate denialOfServiceSanitizerGuard(DataFlow::Node g, Expr e, boolean branch) {
|
||||
exists(DataFlow::Node lesser |
|
||||
e = lesser.asExpr() and
|
||||
g.(DataFlow::RelationalComparisonNode).leq(branch, lesser, _, _) and
|
||||
not e.isConst()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Module for defining predicates and tracking taint flow related to denial of service issues.
|
||||
*/
|
||||
module Config implements DataFlow::ConfigSig {
|
||||
predicate isSource(DataFlow::Node source) { source instanceof UntrustedFlowSource }
|
||||
|
||||
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
exists(Function f, DataFlow::CallNode cn | cn = f.getACall() |
|
||||
f.hasQualifiedName("strconv", ["Atoi", "ParseInt", "ParseUint", "ParseFloat"]) and
|
||||
node1 = cn.getArgument(0) and
|
||||
node2 = cn.getResult(0)
|
||||
)
|
||||
}
|
||||
|
||||
predicate isBarrier(DataFlow::Node node) {
|
||||
node = DataFlow::BarrierGuard<denialOfServiceSanitizerGuard/3>::getABarrierNode()
|
||||
}
|
||||
|
||||
predicate isSink(DataFlow::Node sink) { sink = Builtin::make().getACall().getArgument(0) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Tracks taint flow for reasoning about denial of service, where source is
|
||||
* user-controlled and unchecked.
|
||||
*/
|
||||
module Flow = TaintTracking::Global<Config>;
|
||||
|
||||
import Flow::PathGraph
|
||||
|
||||
from Flow::PathNode source, Flow::PathNode sink
|
||||
where Flow::flowPath(source, sink)
|
||||
select sink, source, sink, "This variable might be leading to denial of service."
|
||||
@@ -1,18 +0,0 @@
|
||||
edges
|
||||
| DenialOfServiceBad.go:11:12:11:16 | selection of URL | DenialOfServiceBad.go:11:12:11:24 | call to Query | provenance | |
|
||||
| DenialOfServiceBad.go:11:12:11:24 | call to Query | DenialOfServiceBad.go:13:15:13:20 | source | provenance | |
|
||||
| DenialOfServiceBad.go:13:15:13:20 | source | DenialOfServiceBad.go:13:15:13:29 | call to Get | provenance | |
|
||||
| DenialOfServiceBad.go:13:15:13:29 | call to Get | DenialOfServiceBad.go:14:28:14:36 | sourceStr | provenance | |
|
||||
| DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | DenialOfServiceBad.go:20:27:20:30 | sink | provenance | |
|
||||
| DenialOfServiceBad.go:14:28:14:36 | sourceStr | DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | provenance | |
|
||||
nodes
|
||||
| DenialOfServiceBad.go:11:12:11:16 | selection of URL | semmle.label | selection of URL |
|
||||
| DenialOfServiceBad.go:11:12:11:24 | call to Query | semmle.label | call to Query |
|
||||
| DenialOfServiceBad.go:13:15:13:20 | source | semmle.label | source |
|
||||
| DenialOfServiceBad.go:13:15:13:29 | call to Get | semmle.label | call to Get |
|
||||
| DenialOfServiceBad.go:14:2:14:37 | ... := ...[0] | semmle.label | ... := ...[0] |
|
||||
| DenialOfServiceBad.go:14:28:14:36 | sourceStr | semmle.label | sourceStr |
|
||||
| DenialOfServiceBad.go:20:27:20:30 | sink | semmle.label | sink |
|
||||
subpaths
|
||||
#select
|
||||
| DenialOfServiceBad.go:20:27:20:30 | sink | DenialOfServiceBad.go:11:12:11:16 | selection of URL | DenialOfServiceBad.go:20:27:20:30 | sink | This variable might be leading to denial of service. |
|
||||
@@ -1 +0,0 @@
|
||||
experimental/CWE-770/DenialOfService.ql
|
||||
@@ -0,0 +1,4 @@
|
||||
import go
|
||||
import semmle.go.security.UncontrolledAllocationSize
|
||||
import TestUtilities.InlineFlowTest
|
||||
import FlowTest<UncontrolledAllocationSize::Config, UncontrolledAllocationSize::Config>
|
||||
@@ -17,7 +17,7 @@ func OutOfMemoryBad(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
result := make([]string, sink)
|
||||
result := make([]string, sink) // $hasTaintFlow="sink"
|
||||
for i := 0; i < sink; i++ {
|
||||
result[i] = fmt.Sprintf("Item %d", i+1)
|
||||
}
|
||||
Reference in New Issue
Block a user