Skip to content
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

Go: new query for detect DOS vulnerability #15130

Merged
merged 13 commits into from
Mar 6, 2024
32 changes: 32 additions & 0 deletions go/ql/src/experimental/CWE-770/DenialOfService.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<!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>
59 changes: 59 additions & 0 deletions go/ql/src/experimental/CWE-770/DenialOfService.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
/**
* @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) {
Malayke marked this conversation as resolved.
Show resolved Hide resolved
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 {
Malayke marked this conversation as resolved.
Show resolved Hide resolved
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."
27 changes: 27 additions & 0 deletions go/ql/src/experimental/CWE-770/DenialOfServiceBad.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package main

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
)

func OutOfMemoryBad(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()

queryStr := query.Get("n")
collectionSize, err := strconv.Atoi(queryStr)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

result := make([]string, collectionSize)
for i := 0; i < collectionSize; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}
30 changes: 30 additions & 0 deletions go/ql/src/experimental/CWE-770/DenialOfServiceGood.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package main

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
)

func OutOfMemoryGood(w http.ResponseWriter, r *http.Request) {
query := r.URL.Query()
MaxValue := 6
queryStr := query.Get("n")
collectionSize, err := strconv.Atoi(queryStr)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
if collectionSize < 0 || collectionSize > MaxValue {
http.Error(w, "Bad request", http.StatusBadRequest)
return
}
result := make([]string, collectionSize)
for i := 0; i < collectionSize; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}
18 changes: 18 additions & 0 deletions go/ql/test/experimental/CWE-770/DenialOfService.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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 change: 1 addition & 0 deletions go/ql/test/experimental/CWE-770/DenialOfService.qlref
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
experimental/CWE-770/DenialOfService.ql
27 changes: 27 additions & 0 deletions go/ql/test/experimental/CWE-770/DenialOfServiceBad.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package main

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
)

func OutOfMemoryBad(w http.ResponseWriter, r *http.Request) {
source := r.URL.Query()

sourceStr := source.Get("n")
sink, err := strconv.Atoi(sourceStr)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}

result := make([]string, sink)
for i := 0; i < sink; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}
94 changes: 94 additions & 0 deletions go/ql/test/experimental/CWE-770/DenialOfServiceGood.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package main

import (
"encoding/json"
"fmt"
"net/http"
"strconv"
)

func OutOfMemoryGood1(w http.ResponseWriter, r *http.Request) {
source := r.URL.Query()
MaxValue := 6
sourceStr := source.Get("n")
sink, err := strconv.Atoi(sourceStr)
if err != nil || sink < 0 {
http.Error(w, "Bad request", http.StatusBadRequest)
return
}
if sink > MaxValue {
return
}
result := make([]string, sink)
for i := 0; i < sink; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}

func OutOfMemoryGood2(w http.ResponseWriter, r *http.Request) {
source := r.URL.Query()
MaxValue := 6
sourceStr := source.Get("n")
sink, err := strconv.Atoi(sourceStr)
if err != nil || sink < 0 {
http.Error(w, "Bad request", http.StatusBadRequest)
return
}
if sink <= MaxValue {
result := make([]string, sink)
for i := 0; i < sink; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}
}

func OutOfMemoryGood3(w http.ResponseWriter, r *http.Request) {
source := r.URL.Query()
MaxValue := 6
sourceStr := source.Get("n")
sink, err := strconv.Atoi(sourceStr)
if err != nil || sink < 0 {
http.Error(w, "Bad request", http.StatusBadRequest)
return
}
if sink > MaxValue {
sink = MaxValue
result := make([]string, sink)
for i := 0; i < sink; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}
}

func OutOfMemoryGood4(w http.ResponseWriter, r *http.Request) {
source := r.URL.Query()
MaxValue := 6
sourceStr := source.Get("n")
sink, err := strconv.Atoi(sourceStr)
if err != nil || sink < 0 {
http.Error(w, "Bad request", http.StatusBadRequest)
return
}
if sink > MaxValue {
sink = MaxValue
} else {
tmp := sink
sink = tmp
}
result := make([]string, sink)
for i := 0; i < sink; i++ {
result[i] = fmt.Sprintf("Item %d", i+1)
}

w.Header().Set("Content-Type", "application/json")
json.NewEncoder(w).Encode(result)
}