Skip to content

Commit

Permalink
Merge pull request #15843 from atorralba/atorralba/go/uncontrolled-al…
Browse files Browse the repository at this point in the history
…location-size

Go: Promote `go/uncontrolled-allocation-size` from experimental
  • Loading branch information
atorralba authored Mar 11, 2024
2 parents 5749fdb + ff2d78d commit 0443620
Show file tree
Hide file tree
Showing 15 changed files with 133 additions and 111 deletions.
34 changes: 34 additions & 0 deletions go/ql/lib/semmle/go/security/UncontrolledAllocationSize.qll
Original file line number Diff line number Diff line change
@@ -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>;
}
Original file line number Diff line number Diff line change
@@ -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 changes: 36 additions & 0 deletions go/ql/src/Security/CWE-770/UncontrolledAllocationSize.qhelp
Original file line number Diff line number Diff line change
@@ -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="UncontrolledAllocationSizeBad.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="UncontrolledAllocationSizeGood.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>
21 changes: 21 additions & 0 deletions go/ql/src/Security/CWE-770/UncontrolledAllocationSize.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* @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 error
* @security-severity 7.5
* @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"
Original file line number Diff line number Diff line change
@@ -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).
32 changes: 0 additions & 32 deletions go/ql/src/experimental/CWE-770/DenialOfService.qhelp

This file was deleted.

59 changes: 0 additions & 59 deletions go/ql/src/experimental/CWE-770/DenialOfService.ql

This file was deleted.

18 changes: 0 additions & 18 deletions go/ql/test/experimental/CWE-770/DenialOfService.expected

This file was deleted.

1 change: 0 additions & 1 deletion go/ql/test/experimental/CWE-770/DenialOfService.qlref

This file was deleted.

Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
import go
import semmle.go.security.UncontrolledAllocationSize
import TestUtilities.InlineFlowTest
import FlowTest<UncontrolledAllocationSize::Config, UncontrolledAllocationSize::Config>
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down

0 comments on commit 0443620

Please sign in to comment.