mirror of
https://github.com/github/codeql.git
synced 2026-05-01 19:55:15 +02:00
Merge pull request #6713 from geoffw0/cwe139
C++: New query for 'Cleartext transmission of sensitive information'
This commit is contained in:
Mathias Vorreiter Pedersen
2
cpp/change-notes/2021-06-10-cleartext-transmission.md
Normal file
2
cpp/change-notes/2021-06-10-cleartext-transmission.md
Normal file
@@ -0,0 +1,2 @@
|
||||
lgtm,codescanning
|
||||
* A new query (`cpp/cleartext-transmission`) has been added. This is similar to the `cpp/cleartext-storage-file`, `cpp/cleartext-storage-buffer` and `cpp/cleartext-storage-database` queries but looks for cases where sensitive information is most likely transmitted over a network.
|
||||
@@ -85,4 +85,6 @@ private class Recv extends AliasFunction, ArrayFunction, SideEffectFunction,
|
||||
) and
|
||||
description = "Buffer read by " + this.getName()
|
||||
}
|
||||
|
||||
override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }
|
||||
}
|
||||
|
||||
@@ -60,4 +60,6 @@ private class Send extends AliasFunction, ArrayFunction, SideEffectFunction, Rem
|
||||
override predicate hasRemoteFlowSink(FunctionInput input, string description) {
|
||||
input.isParameterDeref(1) and description = "Buffer sent by " + this.getName()
|
||||
}
|
||||
|
||||
override predicate hasSocketInput(FunctionInput input) { input.isParameter(0) }
|
||||
}
|
||||
|
||||
@@ -18,6 +18,12 @@ abstract class RemoteFlowSourceFunction extends Function {
|
||||
* Holds if remote data described by `description` flows from `output` of a call to this function.
|
||||
*/
|
||||
abstract predicate hasRemoteFlowSource(FunctionOutput output, string description);
|
||||
|
||||
/**
|
||||
* Holds if remote data from this source comes from a socket described by
|
||||
* `input`. There is no result if a socket is not specified.
|
||||
*/
|
||||
predicate hasSocketInput(FunctionInput input) { none() }
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -51,4 +57,10 @@ abstract class RemoteFlowSinkFunction extends Function {
|
||||
* send over a network connection.
|
||||
*/
|
||||
abstract predicate hasRemoteFlowSink(FunctionInput input, string description);
|
||||
|
||||
/**
|
||||
* Holds if data put into this sink is transmitted through a socket described
|
||||
* by `input`. There is no result if a socket is not specified.
|
||||
*/
|
||||
predicate hasSocketInput(FunctionInput input) { none() }
|
||||
}
|
||||
|
||||
@@ -9,7 +9,7 @@ storage.</p>
|
||||
</overview>
|
||||
<recommendation>
|
||||
|
||||
<p>Ensure that sensitive information is always encrypted before being stored, especially before writing to a file.
|
||||
<p>Ensure that sensitive information is always encrypted before being stored to a file or transmitted over the network.
|
||||
It may be wise to encrypt information before it is put into a buffer that may be readable in memory.</p>
|
||||
|
||||
<p>In general, decrypt sensitive information only at the point where it is necessary for it to be used in
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="CleartextStorage.inc.qhelp" /></qhelp>
|
||||
124
cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql
Normal file
124
cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql
Normal file
@@ -0,0 +1,124 @@
|
||||
/**
|
||||
* @name Cleartext transmission of sensitive information
|
||||
* @description Transmitting sensitive information across a network in
|
||||
* cleartext can expose it to an attacker.
|
||||
* @kind path-problem
|
||||
* @problem.severity warning
|
||||
* @security-severity 7.5
|
||||
* @precision medium
|
||||
* @id cpp/cleartext-transmission
|
||||
* @tags security
|
||||
* external/cwe/cwe-319
|
||||
*/
|
||||
|
||||
import cpp
|
||||
import semmle.code.cpp.security.SensitiveExprs
|
||||
import semmle.code.cpp.dataflow.TaintTracking
|
||||
import semmle.code.cpp.models.interfaces.FlowSource
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* A function call that sends or receives data over a network.
|
||||
*/
|
||||
abstract class NetworkSendRecv extends FunctionCall {
|
||||
/**
|
||||
* Gets the expression for the socket or similar object used for sending or
|
||||
* receiving data (if any).
|
||||
*/
|
||||
abstract Expr getSocketExpr();
|
||||
|
||||
/**
|
||||
* Gets the expression for the buffer to be sent from / received into.
|
||||
*/
|
||||
abstract Expr getDataExpr();
|
||||
}
|
||||
|
||||
/**
|
||||
* A function call that sends data over a network.
|
||||
*
|
||||
* note: functions such as `write` may be writing to a network source or a file. We could attempt to determine which, and sort results into `cpp/cleartext-transmission` and perhaps `cpp/cleartext-storage-file`. In practice it usually isn't very important which query reports a result as long as its reported exactly once.
|
||||
*/
|
||||
class NetworkSend extends NetworkSendRecv {
|
||||
RemoteFlowSinkFunction target;
|
||||
|
||||
NetworkSend() { target = this.getTarget() }
|
||||
|
||||
override Expr getSocketExpr() {
|
||||
exists(FunctionInput input, int arg |
|
||||
target.hasSocketInput(input) and
|
||||
input.isParameter(arg) and
|
||||
result = this.getArgument(arg)
|
||||
)
|
||||
}
|
||||
|
||||
override Expr getDataExpr() {
|
||||
exists(FunctionInput input, int arg |
|
||||
target.hasRemoteFlowSink(input, _) and
|
||||
input.isParameterDeref(arg) and
|
||||
result = this.getArgument(arg)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A function call that receives data over a network.
|
||||
*/
|
||||
class NetworkRecv extends NetworkSendRecv {
|
||||
RemoteFlowSourceFunction target;
|
||||
|
||||
NetworkRecv() { target = this.getTarget() }
|
||||
|
||||
override Expr getSocketExpr() {
|
||||
exists(FunctionInput input, int arg |
|
||||
target.hasSocketInput(input) and
|
||||
input.isParameter(arg) and
|
||||
result = this.getArgument(arg)
|
||||
)
|
||||
}
|
||||
|
||||
override Expr getDataExpr() {
|
||||
exists(FunctionOutput output, int arg |
|
||||
target.hasRemoteFlowSource(output, _) and
|
||||
output.isParameterDeref(arg) and
|
||||
result = this.getArgument(arg)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Taint flow from a sensitive expression to a network operation with data
|
||||
* tainted by that expression.
|
||||
*/
|
||||
class SensitiveSendRecvConfiguration extends TaintTracking::Configuration {
|
||||
SensitiveSendRecvConfiguration() { this = "SensitiveSendRecvConfiguration" }
|
||||
|
||||
override predicate isSource(DataFlow::Node source) { source.asExpr() instanceof SensitiveExpr }
|
||||
|
||||
override predicate isSink(DataFlow::Node sink) {
|
||||
exists(NetworkSendRecv transmission |
|
||||
sink.asExpr() = transmission.getDataExpr() and
|
||||
// a zero socket descriptor is standard input, which is not interesting for this query.
|
||||
not exists(Zero zero |
|
||||
DataFlow::localFlow(DataFlow::exprNode(zero),
|
||||
DataFlow::exprNode(transmission.getSocketExpr()))
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
from
|
||||
SensitiveSendRecvConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
|
||||
NetworkSendRecv transmission, string msg
|
||||
where
|
||||
config.hasFlowPath(source, sink) and
|
||||
sink.getNode().asExpr() = transmission.getDataExpr() and
|
||||
if transmission instanceof NetworkSend
|
||||
then
|
||||
msg =
|
||||
"This operation transmits '" + sink.toString() +
|
||||
"', which may contain unencrypted sensitive data from $@"
|
||||
else
|
||||
msg =
|
||||
"This operation receives into '" + sink.toString() +
|
||||
"', which may put unencrypted sensitive data into $@"
|
||||
select transmission, source, sink, msg, source, source.getNode().asExpr().toString()
|
||||
@@ -0,0 +1,49 @@
|
||||
edges
|
||||
| test3.cpp:68:21:68:29 | password1 | test3.cpp:70:15:70:17 | ptr |
|
||||
| test3.cpp:75:15:75:22 | password | test3.cpp:77:15:77:17 | ptr |
|
||||
| test3.cpp:106:20:106:25 | buffer | test3.cpp:108:14:108:19 | buffer |
|
||||
| test3.cpp:111:28:111:33 | buffer | test3.cpp:113:9:113:14 | buffer |
|
||||
| test3.cpp:120:9:120:23 | global_password | test3.cpp:138:16:138:29 | call to get_global_str |
|
||||
| test3.cpp:128:11:128:18 | password | test3.cpp:106:20:106:25 | buffer |
|
||||
| test3.cpp:132:21:132:22 | call to id | test3.cpp:134:15:134:17 | ptr |
|
||||
| test3.cpp:132:24:132:32 | password1 | test3.cpp:111:28:111:33 | buffer |
|
||||
| test3.cpp:132:24:132:32 | password1 | test3.cpp:132:21:132:22 | call to id |
|
||||
| test3.cpp:138:16:138:29 | call to get_global_str | test3.cpp:140:15:140:18 | data |
|
||||
| test3.cpp:151:19:151:26 | password | test3.cpp:153:15:153:20 | buffer |
|
||||
nodes
|
||||
| test3.cpp:20:15:20:23 | password1 | semmle.label | password1 |
|
||||
| test3.cpp:24:15:24:23 | password2 | semmle.label | password2 |
|
||||
| test3.cpp:41:15:41:22 | password | semmle.label | password |
|
||||
| test3.cpp:49:15:49:22 | password | semmle.label | password |
|
||||
| test3.cpp:68:21:68:29 | password1 | semmle.label | password1 |
|
||||
| test3.cpp:70:15:70:17 | ptr | semmle.label | ptr |
|
||||
| test3.cpp:75:15:75:22 | password | semmle.label | password |
|
||||
| test3.cpp:77:15:77:17 | ptr | semmle.label | ptr |
|
||||
| test3.cpp:95:12:95:19 | password | semmle.label | password |
|
||||
| test3.cpp:106:20:106:25 | buffer | semmle.label | buffer |
|
||||
| test3.cpp:108:14:108:19 | buffer | semmle.label | buffer |
|
||||
| test3.cpp:111:28:111:33 | buffer | semmle.label | buffer |
|
||||
| test3.cpp:113:9:113:14 | buffer | semmle.label | buffer |
|
||||
| test3.cpp:120:9:120:23 | global_password | semmle.label | global_password |
|
||||
| test3.cpp:128:11:128:18 | password | semmle.label | password |
|
||||
| test3.cpp:132:21:132:22 | call to id | semmle.label | call to id |
|
||||
| test3.cpp:132:24:132:32 | password1 | semmle.label | password1 |
|
||||
| test3.cpp:134:15:134:17 | ptr | semmle.label | ptr |
|
||||
| test3.cpp:138:16:138:29 | call to get_global_str | semmle.label | call to get_global_str |
|
||||
| test3.cpp:140:15:140:18 | data | semmle.label | data |
|
||||
| test3.cpp:151:19:151:26 | password | semmle.label | password |
|
||||
| test3.cpp:153:15:153:20 | buffer | semmle.label | buffer |
|
||||
subpaths
|
||||
| test3.cpp:132:24:132:32 | password1 | test3.cpp:111:28:111:33 | buffer | test3.cpp:113:9:113:14 | buffer | test3.cpp:132:21:132:22 | call to id |
|
||||
#select
|
||||
| test3.cpp:20:3:20:6 | call to send | test3.cpp:20:15:20:23 | password1 | test3.cpp:20:15:20:23 | password1 | This operation transmits 'password1', which may contain unencrypted sensitive data from $@ | test3.cpp:20:15:20:23 | password1 | password1 |
|
||||
| test3.cpp:24:3:24:6 | call to send | test3.cpp:24:15:24:23 | password2 | test3.cpp:24:15:24:23 | password2 | This operation transmits 'password2', which may contain unencrypted sensitive data from $@ | test3.cpp:24:15:24:23 | password2 | password2 |
|
||||
| test3.cpp:41:3:41:6 | call to recv | test3.cpp:41:15:41:22 | password | test3.cpp:41:15:41:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:41:15:41:22 | password | password |
|
||||
| test3.cpp:49:3:49:6 | call to recv | test3.cpp:49:15:49:22 | password | test3.cpp:49:15:49:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:49:15:49:22 | password | password |
|
||||
| test3.cpp:70:3:70:6 | call to send | test3.cpp:68:21:68:29 | password1 | test3.cpp:70:15:70:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:68:21:68:29 | password1 | password1 |
|
||||
| test3.cpp:77:3:77:6 | call to recv | test3.cpp:75:15:75:22 | password | test3.cpp:77:15:77:17 | ptr | This operation receives into 'ptr', which may put unencrypted sensitive data into $@ | test3.cpp:75:15:75:22 | password | password |
|
||||
| test3.cpp:95:3:95:6 | call to read | test3.cpp:95:12:95:19 | password | test3.cpp:95:12:95:19 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:95:12:95:19 | password | password |
|
||||
| test3.cpp:108:2:108:5 | call to recv | test3.cpp:128:11:128:18 | password | test3.cpp:108:14:108:19 | buffer | This operation receives into 'buffer', which may put unencrypted sensitive data into $@ | test3.cpp:128:11:128:18 | password | password |
|
||||
| test3.cpp:134:3:134:6 | call to send | test3.cpp:132:24:132:32 | password1 | test3.cpp:134:15:134:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:132:24:132:32 | password1 | password1 |
|
||||
| test3.cpp:140:3:140:6 | call to send | test3.cpp:120:9:120:23 | global_password | test3.cpp:140:15:140:18 | data | This operation transmits 'data', which may contain unencrypted sensitive data from $@ | test3.cpp:120:9:120:23 | global_password | global_password |
|
||||
| test3.cpp:153:3:153:6 | call to send | test3.cpp:151:19:151:26 | password | test3.cpp:153:15:153:20 | buffer | This operation transmits 'buffer', which may contain unencrypted sensitive data from $@ | test3.cpp:151:19:151:26 | password | password |
|
||||
@@ -0,0 +1 @@
|
||||
Security/CWE/CWE-311/CleartextTransmission.ql
|
||||
@@ -0,0 +1,155 @@
|
||||
|
||||
typedef unsigned long size_t;
|
||||
#define STDIN_FILENO (0)
|
||||
|
||||
size_t strlen(const char *s);
|
||||
|
||||
void send(int fd, const void *buf, size_t bufLen, int d);
|
||||
void recv(int fd, void *buf, size_t bufLen, int d);
|
||||
void read(int fd, void *buf, size_t bufLen);
|
||||
|
||||
void LogonUserA(int a, int b, const char *password, int d, int e, int f);
|
||||
|
||||
int val();
|
||||
|
||||
void test_send(const char *password1, const char *password2, const char *password_hash, const char *message)
|
||||
{
|
||||
{
|
||||
LogonUserA(val(), val(), password1, val(), val(), val()); // proof `password1` is plaintext
|
||||
|
||||
send(val(), password1, strlen(password1), val()); // BAD: `password1` is sent plaintext (certainly)
|
||||
}
|
||||
|
||||
{
|
||||
send(val(), password2, strlen(password2), val()); // BAD: `password2` is sent plaintext (probably)
|
||||
}
|
||||
|
||||
{
|
||||
send(val(), password_hash, strlen(password_hash), val()); // GOOD: `password_hash` is sent encrypted
|
||||
}
|
||||
|
||||
{
|
||||
send(val(), message, strlen(message), val()); // GOOD: `message` is not a password
|
||||
}
|
||||
}
|
||||
|
||||
void test_receive()
|
||||
{
|
||||
{
|
||||
char password[256];
|
||||
|
||||
recv(val(), password, 256, val()); // BAD: `password` is received plaintext (certainly)
|
||||
|
||||
LogonUserA(val(), val(), password, val(), val(), val()); // (proof `password` is plaintext)
|
||||
}
|
||||
|
||||
{
|
||||
char password[256];
|
||||
|
||||
recv(val(), password, 256, val()); // BAD: `password` is received plaintext (probably)
|
||||
}
|
||||
|
||||
{
|
||||
char password_hash[256];
|
||||
|
||||
recv(val(), password_hash, 256, val()); // GOOD: `password` is received encrypted
|
||||
}
|
||||
|
||||
{
|
||||
char message[256];
|
||||
|
||||
recv(val(), message, 256, val()); // GOOD: `message` is not a password
|
||||
}
|
||||
}
|
||||
|
||||
void test_dataflow(const char *password1)
|
||||
{
|
||||
{
|
||||
const char *ptr = password1;
|
||||
|
||||
send(val(), ptr, strlen(ptr), val()); // BAD: `password` is sent plaintext
|
||||
}
|
||||
|
||||
{
|
||||
char password[256];
|
||||
char *ptr = password;
|
||||
|
||||
recv(val(), ptr, 256, val()); // BAD: `password` is received plaintext
|
||||
}
|
||||
|
||||
{
|
||||
char buffer[256];
|
||||
|
||||
recv(val(), buffer, 256, val()); // BAD: `password` is received plaintext [NOT DETECTED]
|
||||
|
||||
char *password = buffer;
|
||||
}
|
||||
}
|
||||
|
||||
void test_read()
|
||||
{
|
||||
{
|
||||
char password[256];
|
||||
int fd = val();
|
||||
|
||||
read(fd, password, 256); // BAD: `password` is received plaintext
|
||||
}
|
||||
|
||||
{
|
||||
char password[256];
|
||||
int fd = STDIN_FILENO;
|
||||
|
||||
read(fd, password, 256); // GOOD: `password` is received from stdin, not a network socket
|
||||
}
|
||||
}
|
||||
|
||||
void my_recv(char *buffer, size_t bufferSize)
|
||||
{
|
||||
recv(val(), buffer, bufferSize, val());
|
||||
}
|
||||
|
||||
const char *id(const char *buffer)
|
||||
{
|
||||
return buffer;
|
||||
}
|
||||
|
||||
char *global_password;
|
||||
|
||||
char *get_global_str()
|
||||
{
|
||||
return global_password;
|
||||
}
|
||||
|
||||
void test_interprocedural(const char *password1)
|
||||
{
|
||||
{
|
||||
char password[256];
|
||||
|
||||
my_recv(password, 256); // BAD: `password` is received plaintext [detected on line 108]
|
||||
}
|
||||
|
||||
{
|
||||
const char *ptr = id(password1);
|
||||
|
||||
send(val(), ptr, strlen(ptr), val()); // BAD: `password1` is sent plaintext
|
||||
}
|
||||
|
||||
{
|
||||
char *data = get_global_str();
|
||||
|
||||
send(val(), data, strlen(data), val()); // BAD: `global_password` is sent plaintext
|
||||
}
|
||||
}
|
||||
|
||||
char *strncpy(char *s1, const char *s2, size_t n);
|
||||
|
||||
void test_taint(const char *password)
|
||||
{
|
||||
{
|
||||
char buffer[16];
|
||||
|
||||
strncpy(buffer, password, 16);
|
||||
buffer[15] = 0;
|
||||
send(val(), buffer, 16, val()); // BAD: `password` is (partially) sent plaintext
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user