mirror of
https://github.com/github/codeql.git
synced 2025-12-20 18:56:32 +01:00
C++: Fix results that flow to/from encryption routines.
This commit is contained in:
Geoffrey White
@@ -14,11 +14,14 @@
|
||||
import cpp
|
||||
import semmle.code.cpp.security.SensitiveExprs
|
||||
import semmle.code.cpp.dataflow.TaintTracking
|
||||
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
|
||||
import semmle.code.cpp.models.interfaces.FlowSource
|
||||
import DataFlow::PathGraph
|
||||
|
||||
/**
|
||||
* A function call that sends or receives 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. See `checkSocket` to narrow this down somewhat.
|
||||
*/
|
||||
abstract class NetworkSendRecv extends FunctionCall {
|
||||
/**
|
||||
@@ -31,12 +34,21 @@ abstract class NetworkSendRecv extends FunctionCall {
|
||||
* Gets the expression for the buffer to be sent from / received into.
|
||||
*/
|
||||
abstract Expr getDataExpr();
|
||||
|
||||
/**
|
||||
* Holds if any socket used by this call could be a true network socket.
|
||||
* A zero socket descriptor is standard input, which is not a network
|
||||
* operation.
|
||||
*/
|
||||
predicate checkSocket() {
|
||||
not exists(Zero zero |
|
||||
DataFlow::localFlow(DataFlow::exprNode(zero), DataFlow::exprNode(getSocketExpr()))
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* 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;
|
||||
@@ -86,33 +98,65 @@ class NetworkRecv extends NetworkSendRecv {
|
||||
}
|
||||
|
||||
/**
|
||||
* Taint flow from a sensitive expression to a network operation with data
|
||||
* tainted by that expression.
|
||||
* An expression that is an argument or return value from an encryption or
|
||||
* decryption call.
|
||||
*/
|
||||
class SensitiveSendRecvConfiguration extends TaintTracking::Configuration {
|
||||
SensitiveSendRecvConfiguration() { this = "SensitiveSendRecvConfiguration" }
|
||||
class Encrypted extends Expr {
|
||||
Encrypted() {
|
||||
exists(FunctionCall fc |
|
||||
fc.getTarget().getName().toLowerCase().regexpMatch(".*(crypt|encode|decode).*") and
|
||||
(
|
||||
this = fc or
|
||||
this = fc.getAnArgument()
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Taint flow from a sensitive expression.
|
||||
*/
|
||||
class FromSensitiveConfiguration extends TaintTracking::Configuration {
|
||||
FromSensitiveConfiguration() { this = "FromSensitiveConfiguration" }
|
||||
|
||||
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()))
|
||||
)
|
||||
)
|
||||
sink.asExpr() = any(NetworkSendRecv nsr | nsr.checkSocket()).getDataExpr()
|
||||
or
|
||||
sink.asExpr() instanceof Encrypted
|
||||
}
|
||||
|
||||
override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) {
|
||||
// flow from pre-update to post-update of the source
|
||||
isSource(node1) and
|
||||
node2.(DataFlow::PostUpdateNode).getPreUpdateNode() = node1
|
||||
or
|
||||
// flow from pre-update to post-update of the sink (in case we can reach other sinks)
|
||||
isSink(node1) and
|
||||
node2.(DataFlow::PostUpdateNode).getPreUpdateNode() = node1
|
||||
or
|
||||
// flow through encryption functions to the return value (in case we can reach other sinks)
|
||||
node2.asExpr().(Encrypted).(FunctionCall).getAnArgument() = node1.asExpr()
|
||||
}
|
||||
}
|
||||
|
||||
from
|
||||
SensitiveSendRecvConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
|
||||
NetworkSendRecv transmission, string msg
|
||||
FromSensitiveConfiguration config, DataFlow::PathNode source, DataFlow::PathNode sink,
|
||||
NetworkSendRecv networkSendRecv, string msg
|
||||
where
|
||||
// flow from sensitive -> network data
|
||||
config.hasFlowPath(source, sink) and
|
||||
sink.getNode().asExpr() = transmission.getDataExpr() and
|
||||
if transmission instanceof NetworkSend
|
||||
sink.getNode().asExpr() = networkSendRecv.getDataExpr() and
|
||||
networkSendRecv.checkSocket() and
|
||||
// no flow from sensitive -> evidence of encryption
|
||||
not exists(DataFlow::Node anySource, DataFlow::Node encrypted |
|
||||
config.hasFlow(anySource, sink.getNode()) and
|
||||
config.hasFlow(anySource, encrypted) and
|
||||
encrypted.asExpr() instanceof Encrypted
|
||||
) and
|
||||
// construct result
|
||||
if networkSendRecv instanceof NetworkSend
|
||||
then
|
||||
msg =
|
||||
"This operation transmits '" + sink.toString() +
|
||||
@@ -121,4 +165,4 @@ where
|
||||
msg =
|
||||
"This operation receives into '" + sink.toString() +
|
||||
"', which may put unencrypted sensitive data into $@"
|
||||
select transmission, source, sink, msg, source, source.getNode().asExpr().toString()
|
||||
select networkSendRecv, source, sink, msg, source, source.getNode().asExpr().toString()
|
||||
|
||||
@@ -1,4 +1,10 @@
|
||||
edges
|
||||
| test2.cpp:43:34:43:34 | s [post update] [password] | test2.cpp:63:22:63:22 | s [password] |
|
||||
| test2.cpp:43:36:43:43 | password | test2.cpp:43:36:43:43 | ref arg password |
|
||||
| test2.cpp:43:36:43:43 | ref arg password | test2.cpp:43:34:43:34 | s [post update] [password] |
|
||||
| test2.cpp:63:22:63:22 | s [password] | test2.cpp:63:24:63:31 | password |
|
||||
| test2.cpp:63:22:63:22 | s [password] | test2.cpp:63:24:63:31 | password |
|
||||
| test2.cpp:63:24:63:31 | password | test2.cpp:63:16:63:20 | call to crypt |
|
||||
| test3.cpp:74:21:74:29 | password1 | test3.cpp:76:15:76:17 | ptr |
|
||||
| test3.cpp:81:15:81:22 | password | test3.cpp:83:15:83:17 | ptr |
|
||||
| test3.cpp:112:20:112:25 | buffer | test3.cpp:114:14:114:19 | buffer |
|
||||
@@ -10,7 +16,37 @@ edges
|
||||
| test3.cpp:138:24:138:32 | password1 | test3.cpp:138:21:138:22 | call to id |
|
||||
| test3.cpp:144:16:144:29 | call to get_global_str | test3.cpp:146:15:146:18 | data |
|
||||
| test3.cpp:157:19:157:26 | password | test3.cpp:159:15:159:20 | buffer |
|
||||
| test3.cpp:173:15:173:22 | password | test3.cpp:175:3:175:17 | call to decrypt_inplace |
|
||||
| test3.cpp:173:15:173:22 | password | test3.cpp:175:19:175:26 | password |
|
||||
| test3.cpp:173:15:173:22 | password | test3.cpp:175:19:175:26 | password |
|
||||
| test3.cpp:175:19:175:26 | password | test3.cpp:175:3:175:17 | call to decrypt_inplace |
|
||||
| test3.cpp:181:15:181:22 | password | test3.cpp:184:3:184:17 | call to decrypt_inplace |
|
||||
| test3.cpp:181:15:181:22 | password | test3.cpp:184:19:184:26 | password |
|
||||
| test3.cpp:181:15:181:22 | password | test3.cpp:184:19:184:26 | password |
|
||||
| test3.cpp:184:19:184:26 | password | test3.cpp:184:3:184:17 | call to decrypt_inplace |
|
||||
| test3.cpp:191:15:191:22 | password | test3.cpp:193:18:193:28 | call to rtn_decrypt |
|
||||
| test3.cpp:191:15:191:22 | password | test3.cpp:193:30:193:37 | password |
|
||||
| test3.cpp:191:15:191:22 | password | test3.cpp:193:30:193:37 | password |
|
||||
| test3.cpp:193:30:193:37 | password | test3.cpp:193:18:193:28 | call to rtn_decrypt |
|
||||
| test3.cpp:199:19:199:26 | password | test3.cpp:199:3:199:17 | call to encrypt_inplace |
|
||||
| test3.cpp:199:19:199:26 | password | test3.cpp:201:15:201:22 | password |
|
||||
| test3.cpp:207:19:207:26 | password | test3.cpp:207:3:207:17 | call to encrypt_inplace |
|
||||
| test3.cpp:207:19:207:26 | password | test3.cpp:210:15:210:22 | password |
|
||||
| test3.cpp:217:18:217:28 | call to rtn_encrypt | test3.cpp:219:15:219:26 | password_ptr |
|
||||
| test3.cpp:217:30:217:37 | password | test3.cpp:217:18:217:28 | call to rtn_encrypt |
|
||||
| test3.cpp:217:30:217:37 | password | test3.cpp:217:18:217:28 | call to rtn_encrypt |
|
||||
| test3.cpp:217:30:217:37 | password | test3.cpp:219:15:219:26 | password_ptr |
|
||||
| test3.cpp:241:8:241:15 | password | test3.cpp:242:8:242:15 | password |
|
||||
| test.cpp:48:29:48:39 | thePassword | test.cpp:48:21:48:27 | call to encrypt |
|
||||
| test.cpp:76:29:76:39 | thePassword | test.cpp:76:21:76:27 | call to encrypt |
|
||||
nodes
|
||||
| test2.cpp:43:34:43:34 | s [post update] [password] | semmle.label | s [post update] [password] |
|
||||
| test2.cpp:43:36:43:43 | password | semmle.label | password |
|
||||
| test2.cpp:43:36:43:43 | ref arg password | semmle.label | ref arg password |
|
||||
| test2.cpp:63:16:63:20 | call to crypt | semmle.label | call to crypt |
|
||||
| test2.cpp:63:22:63:22 | s [password] | semmle.label | s [password] |
|
||||
| test2.cpp:63:24:63:31 | password | semmle.label | password |
|
||||
| test2.cpp:63:24:63:31 | password | semmle.label | password |
|
||||
| test3.cpp:22:15:22:23 | password1 | semmle.label | password1 |
|
||||
| test3.cpp:26:15:26:23 | password2 | semmle.label | password2 |
|
||||
| test3.cpp:38:23:38:31 | password2 | semmle.label | password2 |
|
||||
@@ -35,15 +71,44 @@ nodes
|
||||
| test3.cpp:157:19:157:26 | password | semmle.label | password |
|
||||
| test3.cpp:159:15:159:20 | buffer | semmle.label | buffer |
|
||||
| test3.cpp:173:15:173:22 | password | semmle.label | password |
|
||||
| test3.cpp:173:15:173:22 | password | semmle.label | password |
|
||||
| test3.cpp:175:3:175:17 | call to decrypt_inplace | semmle.label | call to decrypt_inplace |
|
||||
| test3.cpp:175:19:175:26 | password | semmle.label | password |
|
||||
| test3.cpp:175:19:175:26 | password | semmle.label | password |
|
||||
| test3.cpp:181:15:181:22 | password | semmle.label | password |
|
||||
| test3.cpp:181:15:181:22 | password | semmle.label | password |
|
||||
| test3.cpp:184:3:184:17 | call to decrypt_inplace | semmle.label | call to decrypt_inplace |
|
||||
| test3.cpp:184:19:184:26 | password | semmle.label | password |
|
||||
| test3.cpp:184:19:184:26 | password | semmle.label | password |
|
||||
| test3.cpp:191:15:191:22 | password | semmle.label | password |
|
||||
| test3.cpp:191:15:191:22 | password | semmle.label | password |
|
||||
| test3.cpp:193:18:193:28 | call to rtn_decrypt | semmle.label | call to rtn_decrypt |
|
||||
| test3.cpp:193:30:193:37 | password | semmle.label | password |
|
||||
| test3.cpp:193:30:193:37 | password | semmle.label | password |
|
||||
| test3.cpp:199:3:199:17 | call to encrypt_inplace | semmle.label | call to encrypt_inplace |
|
||||
| test3.cpp:199:19:199:26 | password | semmle.label | password |
|
||||
| test3.cpp:199:19:199:26 | password | semmle.label | password |
|
||||
| test3.cpp:201:15:201:22 | password | semmle.label | password |
|
||||
| test3.cpp:207:3:207:17 | call to encrypt_inplace | semmle.label | call to encrypt_inplace |
|
||||
| test3.cpp:207:19:207:26 | password | semmle.label | password |
|
||||
| test3.cpp:207:19:207:26 | password | semmle.label | password |
|
||||
| test3.cpp:210:15:210:22 | password | semmle.label | password |
|
||||
| test3.cpp:217:18:217:28 | call to rtn_encrypt | semmle.label | call to rtn_encrypt |
|
||||
| test3.cpp:217:18:217:28 | call to rtn_encrypt | semmle.label | call to rtn_encrypt |
|
||||
| test3.cpp:217:30:217:37 | password | semmle.label | password |
|
||||
| test3.cpp:217:30:217:37 | password | semmle.label | password |
|
||||
| test3.cpp:219:15:219:26 | password_ptr | semmle.label | password_ptr |
|
||||
| test3.cpp:227:22:227:29 | password | semmle.label | password |
|
||||
| test3.cpp:228:26:228:33 | password | semmle.label | password |
|
||||
| test3.cpp:241:8:241:15 | password | semmle.label | password |
|
||||
| test3.cpp:241:8:241:15 | password | semmle.label | password |
|
||||
| test3.cpp:242:8:242:15 | password | semmle.label | password |
|
||||
| test.cpp:48:21:48:27 | call to encrypt | semmle.label | call to encrypt |
|
||||
| test.cpp:48:29:48:39 | thePassword | semmle.label | thePassword |
|
||||
| test.cpp:48:29:48:39 | thePassword | semmle.label | thePassword |
|
||||
| test.cpp:76:21:76:27 | call to encrypt | semmle.label | call to encrypt |
|
||||
| test.cpp:76:29:76:39 | thePassword | semmle.label | thePassword |
|
||||
| test.cpp:76:29:76:39 | thePassword | semmle.label | thePassword |
|
||||
subpaths
|
||||
| test3.cpp:138:24:138:32 | password1 | test3.cpp:117:28:117:33 | buffer | test3.cpp:119:9:119:14 | buffer | test3.cpp:138:21:138:22 | call to id |
|
||||
#select
|
||||
@@ -59,13 +124,8 @@ subpaths
|
||||
| test3.cpp:140:3:140:6 | call to send | test3.cpp:138:24:138:32 | password1 | test3.cpp:140:15:140:17 | ptr | This operation transmits 'ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:138:24:138:32 | password1 | password1 |
|
||||
| test3.cpp:146:3:146:6 | call to send | test3.cpp:126:9:126:23 | global_password | test3.cpp:146:15:146:18 | data | This operation transmits 'data', which may contain unencrypted sensitive data from $@ | test3.cpp:126:9:126:23 | global_password | global_password |
|
||||
| test3.cpp:159:3:159:6 | call to send | test3.cpp:157:19:157:26 | password | test3.cpp:159:15:159:20 | buffer | This operation transmits 'buffer', which may contain unencrypted sensitive data from $@ | test3.cpp:157:19:157:26 | password | password |
|
||||
| test3.cpp:173:3:173:6 | call to recv | test3.cpp:173:15:173:22 | password | test3.cpp:173:15:173:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:173:15:173:22 | password | password |
|
||||
| test3.cpp:181:3:181:6 | call to recv | test3.cpp:181:15:181:22 | password | test3.cpp:181:15:181:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:181:15:181:22 | password | password |
|
||||
| test3.cpp:191:3:191:6 | call to recv | test3.cpp:191:15:191:22 | password | test3.cpp:191:15:191:22 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:191:15:191:22 | password | password |
|
||||
| test3.cpp:201:3:201:6 | call to send | test3.cpp:201:15:201:22 | password | test3.cpp:201:15:201:22 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@ | test3.cpp:201:15:201:22 | password | password |
|
||||
| test3.cpp:210:3:210:6 | call to send | test3.cpp:210:15:210:22 | password | test3.cpp:210:15:210:22 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@ | test3.cpp:210:15:210:22 | password | password |
|
||||
| test3.cpp:219:3:219:6 | call to send | test3.cpp:219:15:219:26 | password_ptr | test3.cpp:219:15:219:26 | password_ptr | This operation transmits 'password_ptr', which may contain unencrypted sensitive data from $@ | test3.cpp:219:15:219:26 | password_ptr | password_ptr |
|
||||
| test3.cpp:227:2:227:5 | call to send | test3.cpp:227:22:227:29 | password | test3.cpp:227:22:227:29 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@ | test3.cpp:227:22:227:29 | password | password |
|
||||
| test3.cpp:228:2:228:5 | call to send | test3.cpp:228:26:228:33 | password | test3.cpp:228:26:228:33 | password | This operation transmits 'password', which may contain unencrypted sensitive data from $@ | test3.cpp:228:26:228:33 | password | password |
|
||||
| test3.cpp:241:2:241:6 | call to fgets | test3.cpp:241:8:241:15 | password | test3.cpp:241:8:241:15 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:241:8:241:15 | password | password |
|
||||
| test3.cpp:242:2:242:6 | call to fgets | test3.cpp:241:8:241:15 | password | test3.cpp:242:8:242:15 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:241:8:241:15 | password | password |
|
||||
| test3.cpp:242:2:242:6 | call to fgets | test3.cpp:242:8:242:15 | password | test3.cpp:242:8:242:15 | password | This operation receives into 'password', which may put unencrypted sensitive data into $@ | test3.cpp:242:8:242:15 | password | password |
|
||||
|
||||
@@ -170,7 +170,7 @@ void test_decrypt()
|
||||
{
|
||||
char password[256];
|
||||
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted
|
||||
|
||||
decrypt_inplace(password); // proof that `password` was in fact encrypted
|
||||
}
|
||||
@@ -178,7 +178,7 @@ void test_decrypt()
|
||||
{
|
||||
char password[256];
|
||||
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted
|
||||
password[255] = 0;
|
||||
|
||||
decrypt_inplace(password); // proof that `password` was in fact encrypted
|
||||
@@ -188,7 +188,7 @@ void test_decrypt()
|
||||
char password[256];
|
||||
char *password_ptr;
|
||||
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
recv(val(), password, 256, val()); // GOOD: password is encrypted
|
||||
|
||||
password_ptr = rtn_decrypt(password); // proof that `password` was in fact encrypted
|
||||
}
|
||||
@@ -198,7 +198,7 @@ void test_decrypt()
|
||||
|
||||
encrypt_inplace(password); // proof that `password` is in fact encrypted
|
||||
|
||||
send(val(), password, strlen(password), val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
send(val(), password, strlen(password), val()); // GOOD: password is encrypted
|
||||
}
|
||||
|
||||
{
|
||||
@@ -207,7 +207,7 @@ void test_decrypt()
|
||||
encrypt_inplace(password); // proof that `password` is in fact encrypted
|
||||
password[255] = 0;
|
||||
|
||||
send(val(), password, strlen(password), val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
send(val(), password, strlen(password), val()); // GOOD: password is encrypted
|
||||
}
|
||||
|
||||
{
|
||||
@@ -216,7 +216,7 @@ void test_decrypt()
|
||||
|
||||
password_ptr = rtn_encrypt(password); // proof that `password` is in fact encrypted
|
||||
|
||||
send(val(), password_ptr, strlen(password_ptr), val()); // GOOD: password is encrypted [FALSE POSITIVE]
|
||||
send(val(), password_ptr, strlen(password_ptr), val()); // GOOD: password is encrypted
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user