mirror of
https://github.com/github/codeql.git
synced 2026-04-30 11:15:13 +02:00
C++: Modernize cpp/system-data-exposure as a path-problem using IR taint, RemoteFlowSinkFunction.
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
* @description Exposing system data or debugging information helps
|
||||
* an adversary learn about the system and form an
|
||||
* attack plan.
|
||||
* @kind problem
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 6.5
|
||||
* @precision medium
|
||||
@@ -14,7 +14,9 @@
|
||||
|
||||
import cpp
|
||||
import semmle.code.cpp.commons.Environment
|
||||
import semmle.code.cpp.security.OutputWrite
|
||||
import semmle.code.cpp.dataflow.TaintTracking
|
||||
import semmle.code.cpp.models.interfaces.FlowSource
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* An element that should not be exposed to an adversary.
|
||||
@@ -24,35 +26,6 @@ abstract class SystemData extends Element {
|
||||
* Gets an expression that is part of this `SystemData`.
|
||||
*/
|
||||
abstract Expr getAnExpr();
|
||||
|
||||
/**
|
||||
* Gets an expression whose value originates from, or is used by,
|
||||
* this `SystemData`.
|
||||
*/
|
||||
Expr getAnExprIndirect() {
|
||||
// direct SystemData
|
||||
result = this.getAnExpr() or
|
||||
// flow via global or member variable (conservative approximation)
|
||||
result = this.getAnAffectedVar().getAnAccess() or
|
||||
// flow via stack variable
|
||||
definitionUsePair(_, this.getAnExprIndirect(), result) or
|
||||
useUsePair(_, this.getAnExprIndirect(), result) or
|
||||
useUsePair(_, result, this.getAnExprIndirect()) or
|
||||
// flow from assigned value to assignment expression
|
||||
result.(AssignExpr).getRValue() = this.getAnExprIndirect()
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a global or member variable that may be affected by this system
|
||||
* data (conservative approximation).
|
||||
*/
|
||||
private Variable getAnAffectedVar() {
|
||||
(
|
||||
result.getAnAssignedValue() = this.getAnExprIndirect() or
|
||||
result.getAnAccess() = this.getAnExprIndirect()
|
||||
) and
|
||||
not result instanceof LocalScopeVariable
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -311,70 +284,23 @@ class RegQuery extends SystemData {
|
||||
override Expr getAnExpr() { regQuery(this, result) }
|
||||
}
|
||||
|
||||
/**
|
||||
* Somewhere data is output.
|
||||
*/
|
||||
abstract class DataOutput extends Element {
|
||||
/**
|
||||
* Get an expression containing data that is output.
|
||||
*/
|
||||
abstract Expr getASource();
|
||||
}
|
||||
class ExposedSystemDataConfiguration extends TaintTracking::Configuration {
|
||||
ExposedSystemDataConfiguration() { this = "ExposedSystemDataConfiguration" }
|
||||
|
||||
/**
|
||||
* Data that is output via standard output or standard error.
|
||||
*/
|
||||
class StandardOutput extends DataOutput instanceof OutputWrite {
|
||||
override Expr getASource() { result = OutputWrite.super.getASource() }
|
||||
}
|
||||
override predicate isSource(DataFlow::Node source) {
|
||||
source.asExpr() = any(SystemData sd).getAnExpr()
|
||||
}
|
||||
|
||||
private predicate socketCallOrIndirect(FunctionCall call) {
|
||||
// direct socket call
|
||||
// int socket(int domain, int type, int protocol);
|
||||
call.getTarget().getName() = "socket"
|
||||
or
|
||||
exists(ReturnStmt rtn |
|
||||
// indirect socket call
|
||||
call.getTarget() = rtn.getEnclosingFunction() and
|
||||
(
|
||||
socketCallOrIndirect(rtn.getExpr()) or
|
||||
socketCallOrIndirect(rtn.getExpr().(VariableAccess).getTarget().getAnAssignedValue())
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(FunctionCall fc, FunctionInput input, int arg |
|
||||
fc.getTarget().(RemoteFlowSinkFunction).hasRemoteFlowSink(input, _) and
|
||||
input.isParameterDeref(arg) and
|
||||
fc.getArgument(arg) = sink.asExpr()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
private predicate socketFileDescriptor(Expr e) {
|
||||
exists(Variable var, FunctionCall socket |
|
||||
socketCallOrIndirect(socket) and
|
||||
var.getAnAssignedValue() = socket and
|
||||
e = var.getAnAccess()
|
||||
)
|
||||
}
|
||||
|
||||
private predicate socketOutput(FunctionCall call, Expr data) {
|
||||
(
|
||||
// ssize_t send(int sockfd, const void *buf, size_t len, int flags);
|
||||
// ssize_t sendto(int sockfd, const void *buf, size_t len, int flags,
|
||||
// const struct sockaddr *dest_addr, socklen_t addrlen);
|
||||
// ssize_t sendmsg(int sockfd, const struct msghdr *msg, int flags);
|
||||
// int write(int handle, void *buffer, int nbyte);
|
||||
call.getTarget().hasGlobalName(["send", "sendto", "sendmsg", "write"]) and
|
||||
data = call.getArgument(1) and
|
||||
socketFileDescriptor(call.getArgument(0))
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Data that is output via a socket.
|
||||
*/
|
||||
class SocketOutput extends DataOutput {
|
||||
SocketOutput() { socketOutput(this, _) }
|
||||
|
||||
override Expr getASource() { socketOutput(this, result) }
|
||||
}
|
||||
|
||||
from SystemData sd, DataOutput ow
|
||||
where
|
||||
sd.getAnExprIndirect() = ow.getASource() or
|
||||
sd.getAnExprIndirect() = ow.getASource().getAChild*()
|
||||
select ow, "This operation exposes system data from $@.", sd, sd.toString()
|
||||
from ExposedSystemDataConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink
|
||||
where config.hasFlowPath(source, sink)
|
||||
select sink, source, sink, "This operation exposes system data from $@.", source,
|
||||
source.getNode().toString()
|
||||
|
||||
@@ -1 +1,4 @@
|
||||
| tests.c:70:9:70:15 | call to fprintf | This operation exposes system data from $@. | tests.c:54:13:54:22 | call to LogonUserA | call to LogonUserA |
|
||||
edges
|
||||
nodes
|
||||
subpaths
|
||||
#select
|
||||
|
||||
@@ -67,6 +67,6 @@ void CWE535_Info_Exposure_Shell_Error__w32_char_01_bad()
|
||||
printLine("Unable to login.");
|
||||
}
|
||||
/* FLAW: Write sensitive data to stderr */
|
||||
fprintf(stderr, "User attempted access with password: %s\n", password);
|
||||
fprintf(stderr, "User attempted access with password: %s\n", password); // [NOT DETECTED]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,12 +1,28 @@
|
||||
| tests2.cpp:58:12:58:12 | call to operator<< | This operation exposes system data from $@. | tests2.cpp:58:15:58:20 | call to getenv | call to getenv |
|
||||
| tests2.cpp:59:25:59:25 | call to operator<< | This operation exposes system data from $@. | tests2.cpp:59:28:59:33 | call to getenv | call to getenv |
|
||||
| tests2.cpp:63:2:63:5 | call to send | This operation exposes system data from $@. | tests2.cpp:63:13:63:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:64:2:64:5 | call to send | This operation exposes system data from $@. | tests2.cpp:64:13:64:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:65:2:65:5 | call to send | This operation exposes system data from $@. | tests2.cpp:65:13:65:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:66:2:66:5 | call to send | This operation exposes system data from $@. | tests2.cpp:66:13:66:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:78:3:78:6 | call to send | This operation exposes system data from $@. | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | call to mysql_get_client_info |
|
||||
| tests2.cpp:80:3:80:6 | call to send | This operation exposes system data from $@. | tests2.cpp:50:23:50:43 | call to mysql_get_client_info | call to mysql_get_client_info |
|
||||
| tests2.cpp:91:3:91:6 | call to send | This operation exposes system data from $@. | tests2.cpp:89:3:89:20 | call to mysql_real_connect | call to mysql_real_connect |
|
||||
| tests2.cpp:100:3:100:6 | call to send | This operation exposes system data from $@. | tests2.cpp:99:8:99:15 | call to getpwuid | call to getpwuid |
|
||||
| tests2.cpp:109:3:109:6 | call to send | This operation exposes system data from $@. | tests2.cpp:107:12:107:17 | call to getenv | call to getenv |
|
||||
| tests2.cpp:110:3:110:6 | call to send | This operation exposes system data from $@. | tests2.cpp:107:12:107:17 | call to getenv | call to getenv |
|
||||
edges
|
||||
| tests2.cpp:76:18:76:38 | call to mysql_get_client_info | tests2.cpp:79:14:79:19 | buffer |
|
||||
| tests2.cpp:107:3:107:4 | c1 [post update] [ptr] | tests2.cpp:109:14:109:15 | c1 [ptr] |
|
||||
| tests2.cpp:107:3:107:36 | ... = ... | tests2.cpp:107:3:107:4 | c1 [post update] [ptr] |
|
||||
| tests2.cpp:107:12:107:17 | call to getenv | tests2.cpp:107:3:107:36 | ... = ... |
|
||||
| tests2.cpp:109:14:109:15 | c1 [ptr] | tests2.cpp:109:17:109:19 | ptr |
|
||||
nodes
|
||||
| tests2.cpp:63:13:63:18 | call to getenv | semmle.label | call to getenv |
|
||||
| tests2.cpp:64:13:64:18 | call to getenv | semmle.label | call to getenv |
|
||||
| tests2.cpp:65:13:65:18 | call to getenv | semmle.label | call to getenv |
|
||||
| tests2.cpp:66:13:66:18 | call to getenv | semmle.label | call to getenv |
|
||||
| tests2.cpp:76:18:76:38 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info |
|
||||
| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | semmle.label | call to mysql_get_client_info |
|
||||
| tests2.cpp:79:14:79:19 | buffer | semmle.label | buffer |
|
||||
| tests2.cpp:107:3:107:4 | c1 [post update] [ptr] | semmle.label | c1 [post update] [ptr] |
|
||||
| tests2.cpp:107:3:107:36 | ... = ... | semmle.label | ... = ... |
|
||||
| tests2.cpp:107:12:107:17 | call to getenv | semmle.label | call to getenv |
|
||||
| tests2.cpp:109:14:109:15 | c1 [ptr] | semmle.label | c1 [ptr] |
|
||||
| tests2.cpp:109:17:109:19 | ptr | semmle.label | ptr |
|
||||
subpaths
|
||||
#select
|
||||
| tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:18 | call to getenv | tests2.cpp:63:13:63:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:63:13:63:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:18 | call to getenv | tests2.cpp:64:13:64:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:64:13:64:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:18 | call to getenv | tests2.cpp:65:13:65:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:65:13:65:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:66:13:66:18 | call to getenv | tests2.cpp:66:13:66:18 | call to getenv | tests2.cpp:66:13:66:18 | call to getenv | This operation exposes system data from $@. | tests2.cpp:66:13:66:18 | call to getenv | call to getenv |
|
||||
| tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | This operation exposes system data from $@. | tests2.cpp:78:14:78:34 | call to mysql_get_client_info | call to mysql_get_client_info |
|
||||
| tests2.cpp:79:14:79:19 | buffer | tests2.cpp:76:18:76:38 | call to mysql_get_client_info | tests2.cpp:79:14:79:19 | buffer | This operation exposes system data from $@. | tests2.cpp:76:18:76:38 | call to mysql_get_client_info | call to mysql_get_client_info |
|
||||
| tests2.cpp:109:17:109:19 | ptr | tests2.cpp:107:12:107:17 | call to getenv | tests2.cpp:109:17:109:19 | ptr | This operation exposes system data from $@. | tests2.cpp:107:12:107:17 | call to getenv | call to getenv |
|
||||
|
||||
@@ -55,8 +55,8 @@ void test1()
|
||||
int sock = socket(val(), val(), val());
|
||||
|
||||
// tests for a strict implementation of CWE-497
|
||||
std::cout << getenv("HOME"); // BAD: outputs HOME environment variable
|
||||
std::cout << "PATH = " << getenv("PATH") << "."; // BAD: outputs PATH environment variable
|
||||
std::cout << getenv("HOME"); // BAD: outputs HOME environment variable [NOT DETECTED]
|
||||
std::cout << "PATH = " << getenv("PATH") << "."; // BAD: outputs PATH environment variable [NOT DETECTED]
|
||||
std::cout << "PATHPATHPATH"; // GOOD: not system data
|
||||
|
||||
// tests for a more pragmatic implementation of CWE-497
|
||||
@@ -76,8 +76,8 @@ void test1()
|
||||
strcpy(buffer, mysql_get_client_info());
|
||||
|
||||
send(sock, mysql_get_client_info(), val(), val()); // BAD
|
||||
send(sock, buffer, val(), val()); // BAD [NOT DETECTED]
|
||||
send(sock, global1, val(), val()); // BAD
|
||||
send(sock, buffer, val(), val()); // BAD
|
||||
send(sock, global1, val(), val()); // BAD [NOT DETECTED]
|
||||
send(sock, global2, val(), val()); // GOOD: not system data
|
||||
}
|
||||
|
||||
@@ -88,7 +88,7 @@ void test1()
|
||||
|
||||
mysql_real_connect(sock, val(), val(), str1, val(), val(), val(), val());
|
||||
|
||||
send(sock, str1, val(), val()); // BAD
|
||||
send(sock, str1, val(), val()); // BAD [NOT DETECTED]
|
||||
send(sock, str2, val(), val()); // GOOD: not system data
|
||||
}
|
||||
|
||||
@@ -97,7 +97,7 @@ void test1()
|
||||
passwd *pw;
|
||||
|
||||
pw = getpwuid(val());
|
||||
send(sock, pw->pw_passwd, val(), val()); // BAD
|
||||
send(sock, pw->pw_passwd, val(), val()); // BAD [NOT DETECTED]
|
||||
}
|
||||
|
||||
// tests for containers
|
||||
@@ -107,6 +107,6 @@ void test1()
|
||||
c1.ptr = getenv("MY_SECRET_TOKEN");
|
||||
c2.ptr = "";
|
||||
send(sock, c1.ptr, val(), val()); // BAD
|
||||
send(sock, c2.ptr, val(), val()); // GOOD: not system data [FALSE POSITIVE]
|
||||
send(sock, c2.ptr, val(), val()); // GOOD: not system data
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user