Skip to content

Commit 0443620

Browse files
authored
Merge pull request github#15843 from atorralba/atorralba/go/uncontrolled-allocation-size
Go: Promote `go/uncontrolled-allocation-size` from experimental
2 parents 5749fdb + ff2d78d commit 0443620

15 files changed

+133
-111
lines changed
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about uncontrolled allocation size issues.
3+
*/
4+
5+
import go
6+
7+
/**
8+
* Provides a taint-tracking flow for reasoning about uncontrolled allocation size issues.
9+
*/
10+
module UncontrolledAllocationSize {
11+
private import UncontrolledAllocationSizeCustomizations::UncontrolledAllocationSize
12+
13+
/**
14+
* Module for defining predicates and tracking taint flow related to uncontrolled allocation size issues.
15+
*/
16+
module Config implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node source) { source instanceof Source }
18+
19+
predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
20+
21+
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
22+
23+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
24+
exists(Function f, DataFlow::CallNode cn | cn = f.getACall() |
25+
f.hasQualifiedName("strconv", ["Atoi", "ParseInt", "ParseUint", "ParseFloat"]) and
26+
node1 = cn.getArgument(0) and
27+
node2 = cn.getResult(0)
28+
)
29+
}
30+
}
31+
32+
/** Tracks taint flow for reasoning about uncontrolled allocation size issues. */
33+
module Flow = TaintTracking::Global<Config>;
34+
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/**
2+
* Provides default sources, sinks, and sanitizers for reasoning about uncontrolled allocation size issues,
3+
* as well as extension points for adding your own.
4+
*/
5+
6+
import go
7+
private import semmle.go.security.AllocationSizeOverflow
8+
9+
/**
10+
* Provides extension points for customizing the taint-tracking configuration for reasoning
11+
* about uncontrolled allocation size issues.
12+
*/
13+
module UncontrolledAllocationSize {
14+
/** A data flow source for uncontrolled allocation size vulnerabilities. */
15+
abstract class Source extends DataFlow::Node { }
16+
17+
/** A data flow sink for uncontrolled allocation size vulnerabilities. */
18+
abstract class Sink extends DataFlow::Node { }
19+
20+
/** A sanitizer for uncontrolled allocation size vulnerabilities. */
21+
abstract class Sanitizer extends DataFlow::Node { }
22+
23+
/** A source of untrusted data, considered as a taint source for uncontrolled size allocation vulnerabilities. */
24+
private class UntrustedFlowAsSource extends Source instanceof UntrustedFlowSource { }
25+
26+
/** The size argument of a memory allocation function. */
27+
private class AllocationSizeAsSink extends Sink instanceof AllocationSizeOverflow::AllocationSize {
28+
}
29+
30+
/** A check that a value is below some upper limit. */
31+
private class SizeCheckSanitizer extends Sanitizer instanceof AllocationSizeOverflow::AllocationSizeCheckBarrier
32+
{ }
33+
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
3+
<qhelp>
4+
<overview>
5+
<p>Using untrusted input to allocate slices with the built-in <code>make</code> function could
6+
lead to excessive memory allocation and potentially cause the program to crash due to running
7+
out of memory. This vulnerability could be exploited to perform a denial-of-service attack by
8+
consuming all available server resources.</p>
9+
</overview>
10+
11+
<recommendation>
12+
<p>Implement a maximum allowed value for size allocations with the built-in <code>make</code>
13+
function to prevent excessively large allocations.</p>
14+
</recommendation>
15+
16+
<example>
17+
<p>In the following example snippet, the <code>n</code> parameter is user-controlled.</p>
18+
<p>If the external user provides an excessively large value, the application allocates a slice
19+
of size <code>n</code> without further verification, potentially exhausting all the available
20+
memory.</p>
21+
22+
<sample src="UncontrolledAllocationSizeBad.go" />
23+
24+
<p>One way to prevent this vulnerability is by implementing a maximum allowed value for the
25+
user-controlled input, as seen in the following example:</p>
26+
27+
<sample src="UncontrolledAllocationSizeGood.go" />
28+
</example>
29+
30+
<references>
31+
<li> OWASP: <a
32+
href="https://cheatsheetseries.owasp.org/cheatsheets/Denial_of_Service_Cheat_Sheet.html">Denial
33+
of Service Cheat Sheet</a>
34+
</li>
35+
</references>
36+
</qhelp>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Slice memory allocation with excessive size value
3+
* @description Allocating memory for slices with the built-in make function from user-controlled sources can lead to a denial of service.
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.5
7+
* @precision high
8+
* @id go/uncontrolled-allocation-size
9+
* @tags security
10+
* external/cwe/cwe-770
11+
*/
12+
13+
import go
14+
import semmle.go.security.UncontrolledAllocationSize
15+
import UncontrolledAllocationSize::Flow::PathGraph
16+
17+
from
18+
UncontrolledAllocationSize::Flow::PathNode source, UncontrolledAllocationSize::Flow::PathNode sink
19+
where UncontrolledAllocationSize::Flow::flowPath(source, sink)
20+
select sink, source, sink, "This memory allocation depends on a $@.", source.getNode(),
21+
"user-provided value"
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* 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).

go/ql/src/experimental/CWE-770/DenialOfService.qhelp

Lines changed: 0 additions & 32 deletions
This file was deleted.

go/ql/src/experimental/CWE-770/DenialOfService.ql

Lines changed: 0 additions & 59 deletions
This file was deleted.

go/ql/test/experimental/CWE-770/DenialOfService.expected

Lines changed: 0 additions & 18 deletions
This file was deleted.

0 commit comments

Comments
 (0)