Skip to content

Navigation Menu

Sign in

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Rust: Recognize more sensitive data sources #19470

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
Loading
from
Open

Rust: Recognize more sensitive data sources #19470

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
87218cb
Rust: Test more examples of sensitive data.
geoffw0 May 8, 2025
8825eef
Rust: More counterexamples for sensitive data as well.
geoffw0 May 8, 2025
a537197
Rust: Understand sensitive field access expressions.
geoffw0 May 9, 2025
0f36e1d
Rust: Understand sensitive qualifier expressions.
geoffw0 May 9, 2025
5f5d6f6
Rust: Understand sensitive enum variants calls.
geoffw0 May 9, 2025
d02d5c5
Rust: Update cleartext logging test with new found results.
geoffw0 May 9, 2025
0a3275e
Rust: One more test case.
geoffw0 May 12, 2025
b907cfe
Rust: Add a few more test cases involving 'map'.
geoffw0 May 12, 2025
ac5ec06
Rust: Constrain SensitiveFieldAccess to avoid including unwanted pare…
geoffw0 May 12, 2025
682f59f
Rust: Make helper predicate private + autoformat.
geoffw0 May 12, 2025
f04d6fd
Rust: Accept minor test changes for the cleartext logging query.
geoffw0 May 12, 2025
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
50 changes: 47 additions & 3 deletions 50 rust/ql/lib/codeql/rust/security/SensitiveData.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ private class SensitiveDataFunction extends Function {
/**
* A function call data flow node that might produce sensitive data.
*/
private class SensitiveDataCall extends SensitiveData {
private class SensitiveDataFunctionCall extends SensitiveData {
SensitiveDataClassification classification;

SensitiveDataCall() {
SensitiveDataFunctionCall() {
Copy link
Contributor

Choose a reason for hiding this comment

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

What about keeping SensitiveDataCall and having its characteristic predicate be:

  SensitiveDataCall() {
    exists(CallExprBase call, string name |
      call = this.asExpr().getExpr() and
      name =
        [
          call.getStaticTarget().(Function).getName().getText(),
          call.(CallExpr).getVariant().getName().getText(),
        ] and
      HeuristicNames::nameIndicatesSensitiveData(name, classification)
    )
  }

?

Then we can remove SensitiveDataFunction, SensitiveDataVariant, and SensitiveDataVariantCall which is quite a bit less code overall.

classification =
this.asExpr()
.getAstNode()
Expand All @@ -53,6 +53,33 @@ private class SensitiveDataCall extends SensitiveData {
override SensitiveDataClassification getClassification() { result = classification }
}

/**
* An enum variant that might produce sensitive data.
*/
private class SensitiveDataVariant extends Variant {
SensitiveDataClassification classification;

SensitiveDataVariant() {
HeuristicNames::nameIndicatesSensitiveData(this.getName().getText(), classification)
}

SensitiveDataClassification getClassification() { result = classification }
}

/**
* An enum variant call data flow node that might produce sensitive data.
*/
private class SensitiveDataVariantCall extends SensitiveData {
SensitiveDataClassification classification;

SensitiveDataVariantCall() {
classification =
this.asExpr().getAstNode().(CallExpr).getVariant().(SensitiveDataVariant).getClassification()
}

override SensitiveDataClassification getClassification() { result = classification }
}

/**
* A variable that might contain sensitive data.
*/
Expand All @@ -67,7 +94,7 @@ private class SensitiveDataVariable extends Variable {
}

/**
* A variable access data flow node that might produce sensitive data.
* A variable access data flow node that might be sensitive data.
*/
private class SensitiveVariableAccess extends SensitiveData {
SensitiveDataClassification classification;
Expand All @@ -84,3 +111,20 @@ private class SensitiveVariableAccess extends SensitiveData {

override SensitiveDataClassification getClassification() { result = classification }
}

private Expr fieldExprParentField(FieldExpr fe) { result = fe.getParentNode() }

/**
* A field access data flow node that might be sensitive data.
*/
private class SensitiveFieldAccess extends SensitiveData {
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved
SensitiveDataClassification classification;

SensitiveFieldAccess() {
exists(FieldExpr fe | fieldExprParentField*(fe) = this.asExpr().getAstNode() |
Copy link
Contributor

Choose a reason for hiding this comment

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

This walks all the way up the expression tree right? How will that work with things like if cond { user.password } else { ... }? I guess we'd like to say that user.password is the sensitive field access but fieldExprParentField also walks up to the if-expression I think.

HeuristicNames::nameIndicatesSensitiveData(fe.getIdentifier().getText(), classification)
)
}

override SensitiveDataClassification getClassification() { result = classification }
}
Loading
Morty Proxy This is a proxified and sanitized view of the page, visit original site.