hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-01-22 19:02:59 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'main' into alexdenisov/macros

Browse Source
This commit is contained in:
Alex Denisov
2023-11-23 19:05:12 +01:00
parent f5ea133067 a8fcfd154f
commit 1b615e25d8
13 changed files with 435 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files

111
cpp/ql/src/Security/CWE/CWE-170/ImproperNullTerminationTainted.ql
View File

@@ -12,79 +12,44 @@
import cpp
import semmle.code.cpp.commons.NullTermination
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
import semmle.code.cpp.security.FlowSources as FS
import semmle.code.cpp.dataflow.new.TaintTracking
import semmle.code.cpp.ir.IR
/** A user-controlled expression that may not be null terminated. */
class TaintSource extends VariableAccess {
TaintSource() {
exists(SecurityOptions x, string cause |
this.getTarget() instanceof SemanticStackVariable and
x.isUserInput(this, cause)
|
cause = ["read", "fread", "recv", "recvfrom", "recvmsg"]
)
}
/**
* Holds if `sink` is a tainted variable access that must be null
* terminated.
*/
private predicate isSink(VariableAccess sink) {
tainted(this, sink) and
variableMustBeNullTerminated(sink)
}
/**
* Holds if this source can reach `va`, possibly using intermediate
* reassignments.
*/
private predicate sourceReaches(VariableAccess va) {
definitionUsePair(_, this, va)
or
exists(VariableAccess mid, Expr def |
this.sourceReaches(mid) and
exprDefinition(_, def, mid) and
definitionUsePair(_, def, va)
)
}
/**
* Holds if the sink `sink` is reachable both from this source and
* from `va`, possibly using intermediate reassignments.
*/
private predicate reachesSink(VariableAccess va, VariableAccess sink) {
this.isSink(sink) and
va = sink
or
exists(VariableAccess mid, Expr def |
this.reachesSink(mid, sink) and
exprDefinition(_, def, va) and
definitionUsePair(_, def, mid)
)
}
/**
* Holds if `sink` is a tainted variable access that must be null
* terminated, and no access which null terminates its contents can
* either reach the sink or be reached from the source. (Ideally,
* we should instead look for such accesses only on the path from
* this source to `sink` found via `tainted(source, sink)`.)
*/
predicate reaches(VariableAccess sink) {
this.isSink(sink) and
not exists(VariableAccess va |
va != this and
va != sink and
mayAddNullTerminator(_, va)
|
this.sourceReaches(va)
or
this.reachesSink(va, sink)
)
}
predicate isSource(FS::FlowSource source, string sourceType) {
sourceType = source.getSourceType() and
exists(VariableAccess va, Call call |
va = source.asDefiningArgument() and
call.getAnArgument() = va and
va.getTarget() instanceof SemanticStackVariable and
call.getTarget().hasGlobalName(["read", "fread", "recv", "recvfrom", "recvmsg"])
)
}
from TaintSource source, VariableAccess sink
where source.reaches(sink)
select sink, "String operation depends on a $@ that may not be null terminated.", source,
"user-provided value"
predicate isSink(DataFlow::Node sink, VariableAccess va) {
va = [sink.asExpr(), sink.asIndirectExpr()] and
variableMustBeNullTerminated(va)
}
private module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { isSource(source, _) }
predicate isBarrier(DataFlow::Node node) {
isSink(node) and node.asExpr().getUnspecifiedType() instanceof ArithmeticType
or
node.asInstruction().(StoreInstruction).getResultType() instanceof ArithmeticType
or
mayAddNullTerminator(_, node.asIndirectExpr())
}
predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
}
module Flow = TaintTracking::Global<Config>;
from DataFlow::Node source, DataFlow::Node sink, VariableAccess va, string sourceType
where
Flow::flow(source, sink) and
isSource(source, sourceType) and
isSink(sink, va)
select va, "String operation depends on $@ that may not be null terminated.", source, sourceType

73
cpp/ql/src/Security/CWE/CWE-807/TaintedCondition.ql
View File

@@ -12,30 +12,83 @@
* external/cwe/cwe-807
*/
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
import TaintedWithPath
import cpp
import semmle.code.cpp.security.Security
import semmle.code.cpp.security.FlowSources
import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.ir.IR
import Flow::PathGraph
Expr getExprWithoutNot(Expr expr) {
result = expr and not expr instanceof NotExpr
or
result = getExprWithoutNot(expr.(NotExpr).getOperand()) and expr instanceof NotExpr
}
predicate sensitiveCondition(Expr condition, Expr raise) {
raisesPrivilege(raise) and
exists(IfStmt ifstmt |
ifstmt.getCondition() = condition and
getExprWithoutNot(ifstmt.getCondition()) = condition and
raise.getEnclosingStmt().getParentStmt*() = ifstmt
)
}
class Configuration extends TaintTrackingConfiguration {
override predicate isSink(Element tainted) { sensitiveCondition(tainted, _) }
private predicate constantInstruction(Instruction instr) {
instr instanceof ConstantInstruction
or
instr instanceof StringConstantInstruction
or
constantInstruction(instr.(UnaryInstruction).getUnary())
}
predicate isSource(FlowSource source, string sourceType) { sourceType = source.getSourceType() }
module Config implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node node) { isSource(node, _) }
predicate isSink(DataFlow::Node node) {
sensitiveCondition([node.asExpr(), node.asIndirectExpr()], _)
}
predicate isBarrier(DataFlow::Node node) {
// Block flow into binary instructions if both operands are non-constant
exists(BinaryInstruction iTo |
iTo = node.asInstruction() and
not constantInstruction(iTo.getLeft()) and
not constantInstruction(iTo.getRight()) and
// propagate taint from either the pointer or the offset, regardless of constant-ness
not iTo instanceof PointerArithmeticInstruction
)
or
// Block flow through calls to pure functions if two or more operands are non-constant
exists(Instruction iFrom1, Instruction iFrom2, CallInstruction iTo |
iTo = node.asInstruction() and
isPureFunction(iTo.getStaticCallTarget().getName()) and
iFrom1 = iTo.getAnArgument() and
iFrom2 = iTo.getAnArgument() and
not constantInstruction(iFrom1) and
not constantInstruction(iFrom2) and
iFrom1 != iFrom2
)
}
}
module Flow = TaintTracking::Global<Config>;
/*
* Produce an alert if there is an 'if' statement whose condition `condition`
* is influenced by tainted data `source`, and the body contains
* `raise` which escalates privilege.
*/
from Expr source, Expr condition, Expr raise, PathNode sourceNode, PathNode sinkNode
from
Expr raise, string sourceType, DataFlow::Node source, DataFlow::Node sink,
Flow::PathNode sourceNode, Flow::PathNode sinkNode
where
taintedWithPath(source, condition, sourceNode, sinkNode) and
sensitiveCondition(condition, raise)
select condition, sourceNode, sinkNode, "Reliance on untrusted input $@ to raise privilege at $@.",
source, source.toString(), raise, raise.toString()
source = sourceNode.getNode() and
sink = sinkNode.getNode() and
isSource(source, sourceType) and
sensitiveCondition([sink.asExpr(), sink.asIndirectExpr()], raise) and
Flow::flowPath(sourceNode, sinkNode)
select sink, sourceNode, sinkNode, "Reliance on $@ to raise privilege at $@.", source, sourceType,
raise, raise.toString()

4
cpp/ql/test/query-tests/Likely Bugs/Memory Management/ImproperNullTermination/ImproperNullTerminationTainted.expected
View File

@@ -1,2 +1,2 @@
| test.cpp:466:10:466:15 | buffer | String operation depends on a $@ that may not be null terminated. | test.cpp:465:18:465:23 | buffer | user-provided value |
| test.cpp:481:10:481:15 | buffer | String operation depends on a $@ that may not be null terminated. | test.cpp:480:9:480:14 | buffer | user-provided value |
| test.cpp:466:10:466:15 | buffer | String operation depends on $@ that may not be null terminated. | test.cpp:465:18:465:23 | read output argument | buffer read by read |
| test.cpp:481:10:481:15 | buffer | String operation depends on $@ that may not be null terminated. | test.cpp:480:9:480:14 | fread output argument | string read by fread |

12
cpp/ql/test/query-tests/Security/CWE/CWE-807/semmle/TaintedCondition/TaintedCondition.expected
View File

@@ -1,13 +1,11 @@
edges
| test.cpp:20:29:20:34 | call to getenv | test.cpp:24:10:24:35 | ! ... |
| test.cpp:20:29:20:34 | call to getenv | test.cpp:24:11:24:16 | call to strcmp |
| test.cpp:20:29:20:47 | call to getenv | test.cpp:24:10:24:35 | ! ... |
| test.cpp:20:29:20:47 | call to getenv | test.cpp:24:11:24:16 | call to strcmp |
subpaths
| test.cpp:20:29:20:47 | call to getenv indirection | test.cpp:24:11:24:16 | call to strcmp |
nodes
| test.cpp:20:29:20:34 | call to getenv | semmle.label | call to getenv |
| test.cpp:20:29:20:47 | call to getenv | semmle.label | call to getenv |
| test.cpp:24:10:24:35 | ! ... | semmle.label | ! ... |
| test.cpp:20:29:20:47 | call to getenv indirection | semmle.label | call to getenv indirection |
| test.cpp:24:11:24:16 | call to strcmp | semmle.label | call to strcmp |
subpaths
#select
| test.cpp:24:10:24:35 | ! ... | test.cpp:20:29:20:34 | call to getenv | test.cpp:24:10:24:35 | ! ... | Reliance on untrusted input $@ to raise privilege at $@. | test.cpp:20:29:20:34 | call to getenv | call to getenv | test.cpp:25:9:25:27 | ... = ... | ... = ... |
| test.cpp:24:11:24:16 | call to strcmp | test.cpp:20:29:20:47 | call to getenv | test.cpp:24:11:24:16 | call to strcmp | Reliance on $@ to raise privilege at $@. | test.cpp:20:29:20:47 | call to getenv | an environment variable | test.cpp:25:9:25:27 | ... = ... | ... = ... |
| test.cpp:24:11:24:16 | call to strcmp | test.cpp:20:29:20:47 | call to getenv indirection | test.cpp:24:11:24:16 | call to strcmp | Reliance on $@ to raise privilege at $@. | test.cpp:20:29:20:47 | call to getenv indirection | an environment variable | test.cpp:25:9:25:27 | ... = ... | ... = ... |

2
go/ql/lib/change-notes/released/0.7.3.md
View File

@@ -2,7 +2,7 @@
### Minor Analysis Improvements
* Added the [gin cors](https://github.com/gin-contrib/cors) library to the CorsMisconfiguration.ql query
* Added the [gin-contrib/cors](https://github.com/gin-contrib/cors) library to the experimental query "CORS misconfiguration" (`go/cors-misconfiguration`).
### Bug Fixes

35
go/ql/src/experimental/CWE-525/WebCacheDeception.qhelp Normal file
View File

@@ -0,0 +1,35 @@
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
<qhelp>
<overview>
<p>
Web Cache Deception is a security vulnerability where an attacker tricks a web server into caching sensitive information and then accesses that cached data.
</p>
<p>
This attack exploits certain behaviors in caching mechanisms by requesting URLs that trick the server into thinking that a non-cachable page is cachable. If a user then accesses sensitive information on these pages, it could be cached and later retrieved by the attacker.
</p>
</overview>
<recommendation>
<p>
To prevent Web Cache Deception attacks, web applications should clearly define cacheable and non-cacheable resources. Implementing strict cache controls and validating requested URLs can mitigate the risk of sensitive data being cached.
</p>
</recommendation>
<example>
<p>
Vulnerable code example: A web server is configured to cache all responses ending in '.css'. An attacker requests 'profile.css', and the server processes 'profile', a sensitive page, and caches it.
</p>
<sample src="WebCacheDeceptionBad.go" />
</example>
<example>
<p>
Secure code example: The server is configured with strict cache controls and URL validation, preventing caching of dynamic or sensitive pages regardless of their URL pattern.
</p>
<sample src="WebCacheDeceptionGood.go" />
</example>
<references>
<li>
OWASP Web Cache Deception Attack:
<a href="https://owasp.org/www-community/attacks/Web_Cache_Deception">Understanding Web Cache Deception Attacks</a>
</li>
<!-- Additional references can be added here -->
</references>
</qhelp>

27
go/ql/src/experimental/CWE-525/WebCacheDeception.ql Normal file
View File

@@ -0,0 +1,27 @@
/*
* @name Web Cache Deception
* @description A caching system has been detected on the application and is vulnerable to web cache deception. By manipulating the URL it is possible to force the application to cache pages that are only accessible by an authenticated user. Once cached, these pages can be accessed by an unauthenticated user.
* @kind problem
* @problem.severity error
* @security-severity 9
* @precision high
* @id go/web-cache-deception
* @tags security
* external/cwe/cwe-525
*/
import go
from
DataFlow::CallNode httpHandleFuncCall, DataFlow::ReadNode rn, Http::HeaderWrite::Range hw,
DeclaredFunction f
where
httpHandleFuncCall.getTarget().hasQualifiedName("net/http", "HandleFunc") and
httpHandleFuncCall.getArgument(0).getStringValue().matches("%/") and
httpHandleFuncCall.getArgument(1) = rn and
rn.reads(f) and
f.getParameter(0) = hw.getResponseWriter() and
hw.getHeaderName() = "cache-control"
select httpHandleFuncCall.getArgument(0),
"Wildcard Endpoint used with " + httpHandleFuncCall.getArgument(0) + " and '" + hw.getHeaderName()
+ "' Header is used"

87
go/ql/src/experimental/CWE-525/WebCacheDeceptionBad.go Normal file
View File

@@ -0,0 +1,87 @@
package bad
import (
"fmt"
"html/template"
"log"
"net/http"
"os/exec"
"strings"
"sync"
)
var sessionMap = make(map[string]string)
var (
templateCache = make(map[string]*template.Template)
mutex = &sync.Mutex{}
)
type Lists struct {
Uid string
UserName string
UserLists []string
ReadFile func(filename string) string
}
func parseTemplateFile(templateName string, tmplFile string) (*template.Template, error) {
mutex.Lock()
defer mutex.Unlock()
// Check if the template is already cached
if cachedTemplate, ok := templateCache[templateName]; ok {
fmt.Println("cached")
return cachedTemplate, nil
}
// Parse and store the template in the cache
parsedTemplate, _ := template.ParseFiles(tmplFile)
fmt.Println("not cached")
templateCache[templateName] = parsedTemplate
return parsedTemplate, nil
}
func ShowAdminPageCache(w http.ResponseWriter, r *http.Request) {
if r.Method == "GET" {
fmt.Println("cache called")
sessionMap[r.RequestURI] = "admin"
// Check if a session value exists
if _, ok := sessionMap[r.RequestURI]; ok {
cmd := "mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in (\"" + "admin" + "\");'"
// mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in ("test");--';echo");'
fmt.Println(cmd)
res, err := exec.Command("sh", "-c", cmd).Output()
if err != nil {
fmt.Println("err : ", err)
}
splitedRes := strings.Split(string(res), "\n")
p := Lists{Uid: "1", UserName: "admin", UserLists: splitedRes}
parsedTemplate, _ := parseTemplateFile("page", "./views/admin/userlists.gtpl")
w.Header().Set("Cache-Control", "no-store, no-cache")
err = parsedTemplate.Execute(w, p)
}
} else {
http.NotFound(w, nil)
}
}
func main() {
fmt.Println("Vulnapp server listening : 1337")
http.Handle("/assets/", http.StripPrefix("/assets/", http.FileServer(http.Dir("assets/"))))
http.HandleFunc("/adminusers/", ShowAdminPageCache)
err := http.ListenAndServe(":1337", nil)
if err != nil {
log.Fatal("ListenAndServe: ", err)
}
}

87
go/ql/src/experimental/CWE-525/WebCacheDeceptionGood.go Normal file
View File

@@ -0,0 +1,87 @@
package good
import (
"fmt"
"html/template"
"log"
"net/http"
"os/exec"
"strings"
"sync"
)
var sessionMap = make(map[string]string)
var (
templateCache = make(map[string]*template.Template)
mutex = &sync.Mutex{}
)
type Lists struct {
Uid string
UserName string
UserLists []string
ReadFile func(filename string) string
}
func parseTemplateFile(templateName string, tmplFile string) (*template.Template, error) {
mutex.Lock()
defer mutex.Unlock()
// Check if the template is already cached
if cachedTemplate, ok := templateCache[templateName]; ok {
fmt.Println("cached")
return cachedTemplate, nil
}
// Parse and store the template in the cache
parsedTemplate, _ := template.ParseFiles(tmplFile)
fmt.Println("not cached")
templateCache[templateName] = parsedTemplate
return parsedTemplate, nil
}
func ShowAdminPageCache(w http.ResponseWriter, r *http.Request) {
if r.Method == "GET" {
fmt.Println("cache called")
sessionMap[r.RequestURI] = "admin"
// Check if a session value exists
if _, ok := sessionMap[r.RequestURI]; ok {
cmd := "mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in (\"" + "admin" + "\");'"
// mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in ("test");--';echo");'
fmt.Println(cmd)
res, err := exec.Command("sh", "-c", cmd).Output()
if err != nil {
fmt.Println("err : ", err)
}
splitedRes := strings.Split(string(res), "\n")
p := Lists{Uid: "1", UserName: "admin", UserLists: splitedRes}
parsedTemplate, _ := parseTemplateFile("page", "./views/admin/userlists.gtpl")
w.Header().Set("Cache-Control", "no-store, no-cache")
err = parsedTemplate.Execute(w, p)
}
} else {
http.NotFound(w, nil)
}
}
func main() {
fmt.Println("Vulnapp server listening : 1337")
http.Handle("/assets/", http.StripPrefix("/assets/", http.FileServer(http.Dir("assets/"))))
http.HandleFunc("/adminusers", ShowAdminPageCache)
err := http.ListenAndServe(":1337", nil)
if err != nil {
log.Fatal("ListenAndServe: ", err)
}
}

1
go/ql/test/experimental/CWE-525/WebCacheDeception.expected Normal file
View File

@@ -0,0 +1 @@
| WebCacheDeceptionBad.go:82:18:82:31 | "/adminusers/" | Wildcard Endpoint used with "/adminusers/" and 'cache-control' Header is used |

1
go/ql/test/experimental/CWE-525/WebCacheDeception.qlref Normal file
View File

@@ -0,0 +1 @@
experimental/CWE-525/WebCacheDeception.ql

87
go/ql/test/experimental/CWE-525/WebCacheDeceptionBad.go Normal file
View File

@@ -0,0 +1,87 @@
package bad
import (
"fmt"
"html/template"
"log"
"net/http"
"os/exec"
"strings"
"sync"
)
var sessionMap = make(map[string]string)
var (
templateCache = make(map[string]*template.Template)
mutex = &sync.Mutex{}
)
type Lists struct {
Uid string
UserName string
UserLists []string
ReadFile func(filename string) string
}
func parseTemplateFile(templateName string, tmplFile string) (*template.Template, error) {
mutex.Lock()
defer mutex.Unlock()
// Check if the template is already cached
if cachedTemplate, ok := templateCache[templateName]; ok {
fmt.Println("cached")
return cachedTemplate, nil
}
// Parse and store the template in the cache
parsedTemplate, _ := template.ParseFiles(tmplFile)
fmt.Println("not cached")
templateCache[templateName] = parsedTemplate
return parsedTemplate, nil
}
func ShowAdminPageCache(w http.ResponseWriter, r *http.Request) {
if r.Method == "GET" {
fmt.Println("cache called")
sessionMap[r.RequestURI] = "admin"
// Check if a session value exists
if _, ok := sessionMap[r.RequestURI]; ok {
cmd := "mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in (\"" + "admin" + "\");'"
// mysql -h mysql -u root -prootwolf -e 'select id,name,mail,age,created_at,updated_at from vulnapp.user where name not in ("test");--';echo");'
fmt.Println(cmd)
res, err := exec.Command("sh", "-c", cmd).Output()
if err != nil {
fmt.Println("err : ", err)
}
splitedRes := strings.Split(string(res), "\n")
p := Lists{Uid: "1", UserName: "admin", UserLists: splitedRes}
parsedTemplate, _ := parseTemplateFile("page", "./views/admin/userlists.gtpl")
w.Header().Set("Cache-Control", "no-store, no-cache")
err = parsedTemplate.Execute(w, p)
}
} else {
http.NotFound(w, nil)
}
}
func main() {
fmt.Println("Vulnapp server listening : 1337")
http.Handle("/assets/", http.StripPrefix("/assets/", http.FileServer(http.Dir("assets/"))))
http.HandleFunc("/adminusers/", ShowAdminPageCache)
err := http.ListenAndServe(":1337", nil)
if err != nil {
log.Fatal("ListenAndServe: ", err)
}
}

2
misc/codegen/generators/qlgen.py
View File

@@ -319,7 +319,7 @@ def _get_stub(cls: schema.Class, base_import: str, generated_import_prefix: str)
_stub_qldoc_header = "// the following QLdoc is generated: if you need to edit it, do it in the schema file\n"
_class_qldoc_re = re.compile(
rf"(?P<qldoc>(?:{re.escape(_stub_qldoc_header)})?/\*\*.*?\*/\s*|^\s*)class\s+(?P<class>\w+)",
rf"(?P<qldoc>(?:{re.escape(_stub_qldoc_header)})?/\*\*.*?\*/\s*|^\s*)(?:class\s+(?P<class>\w+))?",
re.MULTILINE | re.DOTALL)