Skip to content

Commit 5fc8db8

Browse files
authored
Merge pull request #20137 from geoffw0/cleartextstorage
Rust: New Query rust/cleartext-storage-database
2 parents f9f99a0 + 3382d06 commit 5fc8db8

18 files changed

+3166
-96
lines changed

rust/ql/integration-tests/query-suite/rust-code-scanning.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1414
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1515
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
16+
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
1617
ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
1718
ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1819
ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql

rust/ql/integration-tests/query-suite/rust-security-and-quality.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1414
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1515
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
16+
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
1617
ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
1718
ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1819
ql/rust/ql/src/queries/security/CWE-696/BadCtorInitialization.ql

rust/ql/integration-tests/query-suite/rust-security-extended.qls.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ ql/rust/ql/src/queries/security/CWE-022/TaintedPath.ql
1313
ql/rust/ql/src/queries/security/CWE-089/SqlInjection.ql
1414
ql/rust/ql/src/queries/security/CWE-311/CleartextTransmission.ql
1515
ql/rust/ql/src/queries/security/CWE-312/CleartextLogging.ql
16+
ql/rust/ql/src/queries/security/CWE-312/CleartextStorageDatabase.ql
1617
ql/rust/ql/src/queries/security/CWE-327/BrokenCryptoAlgorithm.ql
1718
ql/rust/ql/src/queries/security/CWE-328/WeakSensitiveDataHashing.ql
1819
ql/rust/ql/src/queries/security/CWE-770/UncontrolledAllocationSize.ql
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* Provides classes and predicates for reasoning about cleartext storage
3+
* of sensitive information in a database.
4+
*/
5+
6+
import rust
7+
private import codeql.rust.dataflow.DataFlow
8+
private import codeql.rust.dataflow.internal.DataFlowImpl
9+
private import codeql.rust.security.SensitiveData
10+
private import codeql.rust.Concepts
11+
12+
/**
13+
* Provides default sources, sinks and barriers for detecting cleartext storage
14+
* of sensitive information in a database, as well as extension points for
15+
* adding your own.
16+
*/
17+
module CleartextStorageDatabase {
18+
/**
19+
* A data flow source for cleartext storage vulnerabilities.
20+
*/
21+
abstract class Source extends DataFlow::Node { }
22+
23+
/**
24+
* A data flow sink for cleartext storage vulnerabilities.
25+
*/
26+
abstract class Sink extends QuerySink::Range {
27+
override string getSinkType() { result = "CleartextStorageDatabase" }
28+
}
29+
30+
/**
31+
* A barrier for cleartext storage vulnerabilities.
32+
*/
33+
abstract class Barrier extends DataFlow::Node { }
34+
35+
/**
36+
* Sensitive data, considered as a flow source.
37+
*/
38+
private class SensitiveDataAsSource extends Source instanceof SensitiveData { }
39+
40+
/**
41+
* A sink for cleartext storage vulnerabilities from model data.
42+
* - SQL commands
43+
* - other database storage operations
44+
*/
45+
private class ModelsAsDataSink extends Sink {
46+
ModelsAsDataSink() { sinkNode(this, ["sql-injection", "database-store"]) }
47+
}
48+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: newQuery
3+
---
4+
* Added a new query, `rust/cleartext-storage-database`, for detecting cases where sensitive information is stored non-encrypted in a database.
Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
6+
<overview>
7+
<p>
8+
Sensitive information that is stored unencrypted in a database is accessible to an attacker
9+
who gains access to that database. For example, the information could be accessed by any
10+
process or user in a rooted device, or exposed through another vulnerability.
11+
</p>
12+
</overview>
13+
14+
<recommendation>
15+
<p>
16+
Either encrypt the entire database, or ensure that each piece of sensitive information is
17+
encrypted before being stored. In general, decrypt sensitive information only at the point
18+
where it is necessary for it to be used in cleartext. Avoid storing sensitive information
19+
at all if you do not need to keep it.
20+
</p>
21+
</recommendation>
22+
23+
<example>
24+
<p>
25+
The following example stores sensitive information into a database without encryption, using the
26+
SQLx library:
27+
</p>
28+
<sample src="CleartextStorageDatabaseBad.rs"/>
29+
<p>
30+
This is insecure because the sensitive data is stored in cleartext, making it accessible to anyone
31+
with access to the database.
32+
</p>
33+
<p>
34+
To fix this, we can either encrypt the entire database or encrypt just the sensitive data before it
35+
is stored. Take care to select a secure modern encryption algorithm and put suitable key management
36+
practices into place. In the following example, we have encrypted the sensitive data using 256-bit
37+
AES before storing it in the database:
38+
</p>
39+
<sample src="CleartextStorageDatabaseGood.rs"/>
40+
</example>
41+
42+
<references>
43+
<li>
44+
OWASP Top 10:2021:
45+
<a href="https://owasp.org/Top10/A02_2021-Cryptographic_Failures/">A02:2021 - Cryptographic Failures</a>.
46+
</li>
47+
<li>
48+
OWASP:
49+
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Key_Management_Cheat_Sheet.html">Key Management Cheat Sheet</a>.
50+
</li>
51+
</references>
52+
53+
</qhelp>
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/**
2+
* @name Cleartext storage of sensitive information in a database
3+
* @description Storing sensitive information in a non-encrypted
4+
* database can expose it to an attacker.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 7.5
8+
* @precision high
9+
* @id rust/cleartext-storage-database
10+
* @tags security
11+
* external/cwe/cwe-312
12+
*/
13+
14+
import rust
15+
import codeql.rust.dataflow.DataFlow
16+
import codeql.rust.dataflow.TaintTracking
17+
import codeql.rust.security.CleartextStorageDatabaseExtensions
18+
19+
/**
20+
* A taint configuration from sensitive information to expressions that are
21+
* stored in a database.
22+
*/
23+
module CleartextStorageDatabaseConfig implements DataFlow::ConfigSig {
24+
import CleartextStorageDatabase
25+
26+
predicate isSource(DataFlow::Node node) { node instanceof Source }
27+
28+
predicate isSink(DataFlow::Node node) { node instanceof Sink }
29+
30+
predicate isBarrier(DataFlow::Node barrier) { barrier instanceof Barrier }
31+
32+
predicate isBarrierIn(DataFlow::Node node) {
33+
// make sources barriers so that we only report the closest instance
34+
isSource(node)
35+
}
36+
37+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
38+
// flow from `a` to `&a`
39+
node2.asExpr().getExpr().(RefExpr).getExpr() = node1.asExpr().getExpr()
40+
}
41+
42+
predicate observeDiffInformedIncrementalMode() { any() }
43+
}
44+
45+
module CleartextStorageDatabaseFlow = TaintTracking::Global<CleartextStorageDatabaseConfig>;
46+
47+
import CleartextStorageDatabaseFlow::PathGraph
48+
49+
from
50+
CleartextStorageDatabaseFlow::PathNode sourceNode, CleartextStorageDatabaseFlow::PathNode sinkNode
51+
where CleartextStorageDatabaseFlow::flowPath(sourceNode, sinkNode)
52+
select sinkNode.getNode(), sourceNode, sinkNode,
53+
"This database operation may read or write unencrypted sensitive data from $@.", sourceNode,
54+
sourceNode.getNode().toString()
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
let query = "INSERT INTO PAYMENTDETAILS(ID, CARDNUM) VALUES(?, ?)";
2+
let result = sqlx::query(query)
3+
.bind(id)
4+
.bind(credit_card_number) // BAD: Cleartext storage of sensitive data in the database
5+
.execute(pool)
6+
.await?;
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
fn encrypt(text: String, encryption_key: &aes_gcm::Key<Aes256Gcm>) -> String {
2+
// encrypt text -> ciphertext
3+
let cipher = Aes256Gcm::new(&encryption_key);
4+
let nonce = Aes256Gcm::generate_nonce(&mut OsRng);
5+
let ciphertext = cipher.encrypt(&nonce, text.as_ref()).unwrap();
6+
7+
// append (nonce, ciphertext)
8+
let mut combined = nonce.to_vec();
9+
combined.extend(ciphertext);
10+
11+
// encode to base64 string
12+
BASE64_STANDARD.encode(combined)
13+
}
14+
15+
fn decrypt(data: String, encryption_key: &aes_gcm::Key<Aes256Gcm>) -> String {
16+
let cipher = Aes256Gcm::new(&encryption_key);
17+
18+
// decode base64 string
19+
let decoded = BASE64_STANDARD.decode(data).unwrap();
20+
21+
// split into (nonce, ciphertext)
22+
let nonce_size = <Aes256Gcm as AeadCore>::NonceSize::to_usize();
23+
let (nonce, ciphertext) = decoded.split_at(nonce_size);
24+
25+
// decrypt ciphertext -> plaintext
26+
let plaintext = cipher.decrypt(nonce.into(), ciphertext).unwrap();
27+
String::from_utf8(plaintext).unwrap()
28+
}
29+
30+
...
31+
32+
let encryption_key = Aes256Gcm::generate_key(OsRng);
33+
34+
...
35+
36+
let query = "INSERT INTO PAYMENTDETAILS(ID, CARDNUM) VALUES(?, ?)";
37+
let result = sqlx::query(query)
38+
.bind(id)
39+
.bind(encrypt(credit_card_number, &encryption_key)) // GOOD: Encrypted storage of sensitive data in the database
40+
.execute(pool)
41+
.await?;

rust/ql/src/queries/summary/Stats.qll

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ private import TaintReach
2020
private import codeql.rust.security.regex.RegexInjectionExtensions
2121
private import codeql.rust.security.AccessInvalidPointerExtensions
2222
private import codeql.rust.security.CleartextLoggingExtensions
23+
private import codeql.rust.security.CleartextStorageDatabaseExtensions
2324
private import codeql.rust.security.CleartextTransmissionExtensions
2425
private import codeql.rust.security.SqlInjectionExtensions
2526
private import codeql.rust.security.TaintedPathExtensions

0 commit comments

Comments
 (0)