Skip to content

C++: Fix a FN in cpp/cleartext-storage-file #23019

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.

Sign up for GitHub

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

Merged
MathiasVP merged 5 commits into from
Nov 4, 2026
Merged

C++: Fix a FN in cpp/cleartext-storage-file #23019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1bcea30
C++: Alow char-like types in CWE-311 queries.
MathiasVP Nov 4, 2026
11561b0
C++: Accept test changes.
MathiasVP Nov 4, 2026
03a8c20
C++: Drive-by fix: Select the underlying value instead of the referen…
MathiasVP Nov 4, 2026
37581e4
C++: Accept test changes.
MathiasVP Nov 4, 2026
c1402e6
C++: Add change note.
MathiasVP Nov 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions cpp/ql/src/Security/CWE/CWE-311/ClearText.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
private import cpp
private import semmle.code.cpp.dataflow.new.DataFlow

/**
* Holds if `node` should be a barrier for the CWE-311 queries
* because it is of a non-char integral type.
*/
predicate isBarrier(DataFlow::Node node) {
exists(IntegralType type |
type = node.asCertainDefinition().getUnspecifiedType() and
Comment thread
MathiasVP marked this conversation as resolved.
Resolved
not type instanceof CharType and
not type instanceof WideCharType and
not type instanceof Char8Type and
not type instanceof Char16Type and
not type instanceof Char32Type
)
}

private Expr asExprOrIndirectExpr(DataFlow::Node node, int indirectionIndex) {
indirectionIndex = 0 and
result = node.asExpr()
or
result = node.asIndirectExpr(indirectionIndex)
}

/**
* Holds if `source` is the relevant `DataFlow::Node` for
* expression `e`.
*/
predicate isSource(DataFlow::Node source, Expr e) {
exists(int indirectionIndex |
e = asExprOrIndirectExpr(source, indirectionIndex) and
if e.getUnspecifiedType() instanceof ReferenceType
then indirectionIndex = 1
else indirectionIndex = 0
Comment on lines +32 to +35

@geoffw0 geoffw0 Nov 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason you've written it in this very general way rather than just:

Suggested change
e = asExprOrIndirectExpr(source, indirectionIndex) and
if e.getUnspecifiedType() instanceof ReferenceType
then indirectionIndex = 1
else indirectionIndex = 0
if e.getUnspecifiedType() instanceof ReferenceType
then e = source.asIndirectExpr(1)
else e = source.asExpr()

(which doesn't require asExprOrIndirectExpr to be defined)

@MathiasVP MathiasVP Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would also work, yeah.

)
}
7 changes: 3 additions & 4 deletions cpp/ql/src/Security/CWE/CWE-311/CleartextFileWrite.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import semmle.code.cpp.ir.dataflow.DataFlow
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.ir.dataflow.TaintTracking
import FromSensitiveFlow::PathGraph
import ClearText as ClearText

/**
* A taint flow configuration for flow from a sensitive expression to a `FileWrite` sink.
Expand All @@ -28,9 +29,7 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {

predicate isSink(DataFlow::Node sink) { isSinkImpl(sink, _, _) }

predicate isBarrier(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
predicate isBarrier = ClearText::isBarrier/1;
Comment thread
MathiasVP marked this conversation as resolved.
Resolved

predicate observeDiffInformedIncrementalMode() { any() }

Expand Down Expand Up @@ -61,7 +60,7 @@ predicate isSinkImpl(DataFlow::Node sink, FileWrite w, Expr dest) {

predicate isSourceImpl(DataFlow::Node source, SensitiveExpr sensitive) {
not isFileName(globalValueNumber(sensitive)) and // file names are not passwords
source.asExpr() = sensitive
ClearText::isSource(source, sensitive)
}

/**
Expand Down
23 changes: 9 additions & 14 deletions cpp/ql/src/Security/CWE/CWE-311/CleartextTransmission.ql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import semmle.code.cpp.ir.dataflow.TaintTracking
import semmle.code.cpp.models.interfaces.FlowSource
import semmle.code.cpp.commons.File
import FromSensitiveFlow::PathGraph
import ClearText as ClearText

class SourceVariable extends Variable {
SourceVariable() {
Expand Down Expand Up @@ -218,12 +219,12 @@ predicate isSinkEncrypt(DataFlow::Node sink, Encrypted enc) { sink.asExpr() = en
* function returning sensitive data.
*/
predicate isSourceImpl(DataFlow::Node source) {
exists(VariableAccess e |
e = source.asExpr() and
e.getTarget() instanceof SourceVariable
exists(Expr e |
e.(VariableAccess).getTarget() instanceof SourceVariable or
e.(FunctionCall).getTarget() instanceof SourceFunction
|
ClearText::isSource(source, e)
)
or
source.asExpr().(FunctionCall).getTarget() instanceof SourceFunction
}

/**
Expand All @@ -235,9 +236,7 @@ module FromSensitiveConfig implements DataFlow::ConfigSig {

predicate isSink(DataFlow::Node sink) { isSinkSendRecv(sink, _) }

predicate isBarrier(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
predicate isBarrier = ClearText::isBarrier/1;

predicate isBarrierIn(DataFlow::Node node) {
// As any use of a sensitive variable is a potential source, we need to block flow into
Expand Down Expand Up @@ -266,9 +265,7 @@ module ToEncryptionConfig implements DataFlow::ConfigSig {

predicate isSink(DataFlow::Node sink) { isSinkEncrypt(sink, _) }

predicate isBarrier(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
predicate isBarrier = ClearText::isBarrier/1;

predicate isBarrierIn(DataFlow::Node node) {
// As any use of a sensitive variable is a potential source, we need to block flow into
Expand All @@ -291,9 +288,7 @@ module FromEncryptionConfig implements DataFlow::ConfigSig {

predicate isSink(DataFlow::Node sink) { FromSensitiveFlow::flowTo(sink) }

predicate isBarrier(DataFlow::Node node) {
node.asExpr().getUnspecifiedType() instanceof IntegralType
}
predicate isBarrier = ClearText::isBarrier/1;

predicate observeDiffInformedIncrementalMode() {
none() // only used negatively
Expand Down
4 changes: 4 additions & 0 deletions cpp/ql/src/change-notes/2026-11-04-char-types-cwe-311.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cpp/cleartext-transmission` and `cpp/cleartext-storage-file` queries now track flow through character types. As a result, these queries may find additional results.
40 changes: 35 additions & 5 deletions cpp/ql/test/query-tests/Security/CWE/CWE-311/semmle/tests/CleartextFileWrite.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,24 +10,43 @@
| test2.cpp:73:3:73:9 | call to fprintf | test2.cpp:72:15:72:24 | password | test2.cpp:73:30:73:32 | *buf | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:72:17:72:24 | password | this source. |
| test2.cpp:99:3:99:9 | call to fprintf | test2.cpp:98:45:98:52 | password | test2.cpp:99:27:99:32 | *buffer | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:98:45:98:52 | password | this source. |
| test2.cpp:154:2:154:8 | call to fprintf | test2.cpp:153:31:153:38 | password | test2.cpp:154:21:154:22 | *pw | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:153:31:153:38 | password | this source. |
| test2.cpp:174:2:174:8 | call to fprintf | test2.cpp:173:28:173:35 | password | test2.cpp:174:26:174:30 | *call to c_str | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:173:28:173:35 | password | this source. |
| test2.cpp:168:2:168:8 | call to fprintf | test2.cpp:167:41:167:60 | password | test2.cpp:168:30:168:34 | *call to c_str | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:167:53:167:60 | password | this source. |
| test2.cpp:174:2:174:8 | call to fprintf | test2.cpp:173:28:173:35 | *password | test2.cpp:174:26:174:30 | *call to c_str | This write into file 'log' may contain unencrypted data from $@. | test2.cpp:173:28:173:35 | password | this source. |
| test.cpp:45:3:45:7 | call to fputs | test.cpp:45:9:45:19 | thePassword | test.cpp:45:9:45:19 | thePassword | This write into file 'file' may contain unencrypted data from $@. | test.cpp:45:9:45:19 | thePassword | this source. |
| test.cpp:70:35:70:35 | call to operator<< | test.cpp:70:38:70:48 | thePassword | test.cpp:70:38:70:48 | thePassword | This write into file 'mystream' may contain unencrypted data from $@. | test.cpp:70:38:70:48 | thePassword | this source. |
| test.cpp:73:37:73:41 | call to write | test.cpp:70:38:70:48 | thePassword | test.cpp:73:43:73:53 | thePassword | This write into file 'mystream' may contain unencrypted data from $@. | test.cpp:70:38:70:48 | thePassword | this source. |
| test.cpp:73:37:73:41 | call to write | test.cpp:73:43:73:53 | thePassword | test.cpp:73:43:73:53 | thePassword | This write into file 'mystream' may contain unencrypted data from $@. | test.cpp:73:43:73:53 | thePassword | this source. |
| test.cpp:73:37:73:41 | call to write | test.cpp:73:63:73:73 | thePassword | test.cpp:73:43:73:53 | thePassword | This write into file 'mystream' may contain unencrypted data from $@. | test.cpp:73:63:73:73 | thePassword | this source. |
edges
| file://:0:0:0:0 | * ... | test2.cpp:162:9:162:14 | result | provenance | TaintFunction |
| file://:0:0:0:0 | *++ ... | file://:0:0:0:0 | * ... | provenance | |
| file://:0:0:0:0 | ++ ... | file://:0:0:0:0 | * ... | provenance | |
| file://:0:0:0:0 | ++ ... | file://:0:0:0:0 | *++ ... | provenance | |
| test2.cpp:62:18:62:25 | password | test2.cpp:62:18:62:25 | password | provenance | |
| test2.cpp:62:18:62:25 | password | test2.cpp:65:31:65:34 | cpy1 | provenance | |
| test2.cpp:72:15:72:24 | password | test2.cpp:73:30:73:32 | *buf | provenance | DataFlowFunction |
| test2.cpp:98:45:98:52 | password | test2.cpp:99:27:99:32 | *buffer | provenance | TaintFunction |
| test2.cpp:153:31:153:38 | password | test2.cpp:153:40:153:44 | *call to c_str | provenance | TaintFunction |
| test2.cpp:153:40:153:44 | *call to c_str | test2.cpp:154:21:154:22 | *pw | provenance | |
| test2.cpp:173:28:173:35 | password | test2.cpp:173:28:173:35 | password | provenance | |
| test2.cpp:173:28:173:35 | password | test2.cpp:174:26:174:30 | *call to c_str | provenance | TaintFunction |
| test2.cpp:157:41:157:41 | s | test2.cpp:160:17:160:17 | *s | provenance | |
| test2.cpp:160:17:160:17 | *s | test2.cpp:160:17:160:17 | call to begin | provenance | TaintFunction |
| test2.cpp:160:17:160:17 | call to begin | file://:0:0:0:0 | * ... | provenance | |
| test2.cpp:160:17:160:17 | call to begin | file://:0:0:0:0 | *++ ... | provenance | |
| test2.cpp:160:17:160:17 | call to begin | file://:0:0:0:0 | ++ ... | provenance | |
| test2.cpp:162:9:162:14 | result | test2.cpp:157:13:157:27 | *copy_characters | provenance | |
| test2.cpp:167:25:167:39 | call to copy_characters | test2.cpp:167:25:167:39 | call to copy_characters | provenance | |
| test2.cpp:167:25:167:39 | call to copy_characters | test2.cpp:168:30:168:34 | *call to c_str | provenance | TaintFunction |
| test2.cpp:167:41:167:60 | password | test2.cpp:157:41:157:41 | s | provenance | |
| test2.cpp:167:41:167:60 | password | test2.cpp:167:25:167:39 | call to copy_characters | provenance | TaintFunction |
| test2.cpp:167:41:167:60 | password | test2.cpp:167:41:167:60 | password | provenance | |
| test2.cpp:173:28:173:35 | *password | test2.cpp:173:28:173:35 | *password | provenance | |
| test2.cpp:173:28:173:35 | *password | test2.cpp:174:26:174:30 | *call to c_str | provenance | TaintFunction |
| test.cpp:70:38:70:48 | thePassword | test.cpp:73:43:73:53 | thePassword | provenance | |
| test.cpp:73:63:73:73 | thePassword | test.cpp:73:43:73:53 | thePassword | provenance | |
nodes
| file://:0:0:0:0 | * ... | semmle.label | * ... |
| file://:0:0:0:0 | *++ ... | semmle.label | *++ ... |
| file://:0:0:0:0 | ++ ... | semmle.label | ++ ... |
| test2.cpp:43:36:43:43 | password | semmle.label | password |
| test2.cpp:44:37:44:45 | thepasswd | semmle.label | thepasswd |
| test2.cpp:45:38:45:47 | accountkey | semmle.label | accountkey |
Expand All @@ -45,12 +64,23 @@ nodes
| test2.cpp:153:31:153:38 | password | semmle.label | password |
| test2.cpp:153:40:153:44 | *call to c_str | semmle.label | *call to c_str |
| test2.cpp:154:21:154:22 | *pw | semmle.label | *pw |
| test2.cpp:173:28:173:35 | password | semmle.label | password |
| test2.cpp:173:28:173:35 | password | semmle.label | password |
| test2.cpp:157:13:157:27 | *copy_characters | semmle.label | *copy_characters |
| test2.cpp:157:41:157:41 | s | semmle.label | s |
| test2.cpp:160:17:160:17 | *s | semmle.label | *s |
| test2.cpp:160:17:160:17 | call to begin | semmle.label | call to begin |
| test2.cpp:162:9:162:14 | result | semmle.label | result |
| test2.cpp:167:25:167:39 | call to copy_characters | semmle.label | call to copy_characters |
| test2.cpp:167:25:167:39 | call to copy_characters | semmle.label | call to copy_characters |
| test2.cpp:167:41:167:60 | password | semmle.label | password |
| test2.cpp:167:41:167:60 | password | semmle.label | password |
| test2.cpp:168:30:168:34 | *call to c_str | semmle.label | *call to c_str |
| test2.cpp:173:28:173:35 | *password | semmle.label | *password |
| test2.cpp:173:28:173:35 | *password | semmle.label | *password |
| test2.cpp:174:26:174:30 | *call to c_str | semmle.label | *call to c_str |
| test.cpp:45:9:45:19 | thePassword | semmle.label | thePassword |
| test.cpp:70:38:70:48 | thePassword | semmle.label | thePassword |
| test.cpp:70:38:70:48 | thePassword | semmle.label | thePassword |
| test.cpp:73:43:73:53 | thePassword | semmle.label | thePassword |
| test.cpp:73:63:73:73 | thePassword | semmle.label | thePassword |
subpaths
| test2.cpp:167:41:167:60 | password | test2.cpp:157:41:157:41 | s | test2.cpp:157:13:157:27 | *copy_characters | test2.cpp:167:25:167:39 | call to copy_characters |
Loading
Loading