diff --git a/rust/ql/lib/codeql/rust/security/SensitiveData.qll b/rust/ql/lib/codeql/rust/security/SensitiveData.qll index 5a5f5a6dec03..bf3364abdb6b 100644 --- a/rust/ql/lib/codeql/rust/security/SensitiveData.qll +++ b/rust/ql/lib/codeql/rust/security/SensitiveData.qll @@ -22,64 +22,56 @@ abstract class SensitiveData extends DataFlow::Node { } /** - * A function that might produce sensitive data. - */ -private class SensitiveDataFunction extends Function { - SensitiveDataClassification classification; - - SensitiveDataFunction() { - HeuristicNames::nameIndicatesSensitiveData(this.getName().getText(), classification) - } - - SensitiveDataClassification getClassification() { result = classification } -} - -/** - * A function call data flow node that might produce sensitive data. + * A function call or enum variant data flow node that might produce sensitive data. */ private class SensitiveDataCall extends SensitiveData { SensitiveDataClassification classification; SensitiveDataCall() { - classification = - this.asExpr() - .getAstNode() - .(CallExprBase) - .getStaticTarget() - .(SensitiveDataFunction) - .getClassification() + 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) + ) } override SensitiveDataClassification getClassification() { result = classification } } /** - * A variable that might contain sensitive data. + * A variable access data flow node that might be sensitive data. */ -private class SensitiveDataVariable extends Variable { +private class SensitiveVariableAccess extends SensitiveData { SensitiveDataClassification classification; - SensitiveDataVariable() { - HeuristicNames::nameIndicatesSensitiveData(this.getText(), classification) + SensitiveVariableAccess() { + HeuristicNames::nameIndicatesSensitiveData(this.asExpr() + .getExpr() + .(VariableAccess) + .getVariable() + .(Variable) + .getText(), classification) } - SensitiveDataClassification getClassification() { result = classification } + override SensitiveDataClassification getClassification() { result = classification } } +private Expr fieldExprParentField(FieldExpr fe) { result = fe.getParentNode() } + /** - * A variable access data flow node that might produce sensitive data. + * A field access data flow node that might be sensitive data. */ -private class SensitiveVariableAccess extends SensitiveData { +private class SensitiveFieldAccess extends SensitiveData { SensitiveDataClassification classification; - SensitiveVariableAccess() { - classification = - this.asExpr() - .getAstNode() - .(VariableAccess) - .getVariable() - .(SensitiveDataVariable) - .getClassification() + SensitiveFieldAccess() { + exists(FieldExpr fe | fieldExprParentField*(fe) = this.asExpr().getExpr() | + HeuristicNames::nameIndicatesSensitiveData(fe.getIdentifier().getText(), classification) + ) } override SensitiveDataClassification getClassification() { result = classification } diff --git a/rust/ql/test/library-tests/sensitivedata/test.rs b/rust/ql/test/library-tests/sensitivedata/test.rs index ff5496fb7361..f74de9f5bf86 100644 --- a/rust/ql/test/library-tests/sensitivedata/test.rs +++ b/rust/ql/test/library-tests/sensitivedata/test.rs @@ -10,6 +10,8 @@ struct MyStruct { password: String, password_file_path: String, password_enabled: String, + mfa: String, + numfailed: String, } impl MyStruct { @@ -22,12 +24,14 @@ fn get_password() -> String { get_string() } fn test_passwords( password: &str, pass_word: &str, passwd: &str, my_password: &str, password_str: &str, - pass_phrase: &str, passphrase: &str, passPhrase: &str, - auth_key: &str, authkey: &str, authKey: &str, authentication_key: &str, authenticationkey: &str, authenticationKey: &str, - harmless: &str, encrypted_password: &str, password_hash: &str, + pass_phrase: &str, passphrase: &str, passPhrase: &str, backup_code: &str, + auth_key: &str, authkey: &str, authKey: &str, authentication_key: &str, authenticationkey: &str, authenticationKey: &str, oauth: &str, + one_time_code: &str, + harmless: &str, encrypted_password: &str, password_hash: &str, passwordFile: &str, ms: &MyStruct ) { // passwords + sink(password); // $ sensitive=password sink(pass_word); // $ MISSING: sensitive=password sink(passwd); // $ sensitive=password @@ -36,6 +40,7 @@ fn test_passwords( sink(pass_phrase); // $ sensitive=password sink(passphrase); // $ sensitive=password sink(passPhrase); // $ sensitive=password + sink(backup_code); // $ MISSING: sensitive=password sink(auth_key); // $ sensitive=password sink(authkey); // $ sensitive=password @@ -43,22 +48,31 @@ fn test_passwords( sink(authentication_key); // $ sensitive=password sink(authenticationkey); // $ sensitive=password sink(authenticationKey); // $ sensitive=password + sink(oauth); // $ MISSING: sensitive=password + sink(one_time_code); // $ MISSING: sensitive=password sink(ms); // $ MISSING: sensitive=password - sink(ms.password.as_str()); // $ MISSING: sensitive=password + sink(ms.password.as_str()); // $ sensitive=password + sink(ms.mfa.as_str()); // $ MISSING: sensitive=password sink(get_password()); // $ sensitive=password let password2 = get_string(); sink(password2); // $ sensitive=password + let qry = "password=abc"; + sink(qry); // $ MISSING: sensitive=password + // not passwords + sink(harmless); sink(encrypted_password); sink(password_hash); + sink(passwordFile); // $ SPURIOUS: sensitive=password sink(ms.harmless.as_str()); - sink(ms.password_file_path.as_str()); - sink(ms.password_enabled.as_str()); + sink(ms.password_file_path.as_str()); // $ SPURIOUS: sensitive=password + sink(ms.password_enabled.as_str()); // $ SPURIOUS: sensitive=password + sink(ms.numfailed.as_str()); sink(get_string()); let harmless2 = get_string(); @@ -75,10 +89,11 @@ fn get_next_token() -> String { get_string() } fn test_credentials( account_key: &str, accnt_key: &str, license_key: &str, secret_key: &str, is_secret: bool, num_accounts: i64, username: String, user_name: String, userid: i64, user_id: i64, my_user_id_64: i64, unique_id: i64, uid: i64, - sessionkey: &[u64; 4], session_key: &[u64; 4], hashkey: &[u64; 4], hash_key: &[u64; 4], + sessionkey: &[u64; 4], session_key: &[u64; 4], hashkey: &[u64; 4], hash_key: &[u64; 4], sessionkeypath: &[u64; 4], account_key_path: &[u64; 4], ms: &MyStruct ) { // credentials + sink(account_key); // $ sensitive=id sink(accnt_key); // $ sensitive=id sink(license_key); // $ MISSING: sensitive=secret @@ -101,12 +116,15 @@ fn test_credentials( sink(get_secret_token()); // $ sensitive=secret // not (necessarily) credentials + sink(is_secret); sink(num_accounts); // $ SPURIOUS: sensitive=id sink(unique_id); sink(uid); // $ SPURIOUS: sensitive=id sink(hashkey); sink(hash_key); + sink(sessionkeypath); // $ SPURIOUS: sensitive=id + sink(account_key_path); // $ SPURIOUS: sensitive=id sink(ms.get_certificate_url()); // $ SPURIOUS: sensitive=certificate sink(ms.get_certificate_file()); // $ SPURIOUS: sensitive=certificate @@ -115,58 +133,215 @@ fn test_credentials( sink(get_next_token()); } +struct MacAddr { + values: [u8;12], +} + +struct DeviceInfo { + api_key: String, + deviceApiToken: String, + finger_print: String, + ip_address: String, + macaddr12: [u8;12], + mac_addr: MacAddr, + networkMacAddress: String, + + // not private device info + macro_value: bool, + mac_command: u32, + skip_address: String, +} + +impl DeviceInfo { + fn test_device_info(&self, other: &DeviceInfo) { + // private device info + + sink(&self.api_key); // $ MISSING: sensitive=id + sink(&other.api_key); // $ MISSING: sensitive=id + sink(&self.deviceApiToken); // $ MISSING: sensitive=id + sink(&self.finger_print); // $ MISSING: sensitive=id + sink(&self.ip_address); // $ MISSING: sensitive=id + sink(self.macaddr12); // $ MISSING: sensitive=id + sink(&self.mac_addr); // $ MISSING: sensitive=id + sink(self.mac_addr.values); // $ MISSING: sensitive=id + sink(self.mac_addr.values[0]); // $ MISSING: sensitive=id + sink(&self.networkMacAddress); // $ MISSING: sensitive=id + + // not private device info + + sink(self.macro_value); + sink(self.mac_command); + sink(&self.skip_address); + } +} + struct Financials { harmless: String, my_bank_account_number: String, credit_card_no: String, credit_rating: i32, - user_ccn: String + user_ccn: String, + cvv: String, + beneficiary: String, + routing_number: u64, + routingNumberText: String, + iban: String, + iBAN: String, + + num_accounts: i32, + total_accounts: i32, + accounting: i32, + unaccounted: bool, + multiband: bool, +} + +enum Gender { + Male, + Female, +} + +struct SSN { + data: u128, +} + +impl SSN { + fn get_data(&self) -> u128 { + return self.data; + } } struct MyPrivateInfo { mobile_phone_num: String, contact_email: String, contact_e_mail_2: String, + emergency_contact: String, my_ssn: String, + ssn: SSN, birthday: String, - emergency_contact: String, name_of_employer: String, + gender: Gender, + genderString: String, + + patient_id: u64, + linkedPatientId: u64, + patient_record: String, medical_notes: Vec, + confidentialMessage: String, + latitude: f64, longitude: Option, financials: Financials } +enum ContactDetails { + HomePhoneNumber(String), + MobileNumber(String), + Email(String), + FavouriteColor(String), +} + +struct ContactDetails2 { + home_phone_number: String, +} + fn test_private_info( - info: &MyPrivateInfo + info: &MyPrivateInfo, details: &ContactDetails, ) { // private info - sink(info.mobile_phone_num.as_str()); // $ MISSING: sensitive=private - sink(info.mobile_phone_num.to_string()); // $ MISSING: sensitive=private + + sink(info.mobile_phone_num.as_str()); // $ sensitive=private + sink(info.mobile_phone_num.to_string()); // $ sensitive=private sink(info.contact_email.as_str()); // $ MISSING: sensitive=private sink(info.contact_e_mail_2.as_str()); // $ MISSING: sensitive=private - sink(info.my_ssn.as_str()); // $ MISSING: sensitive=private - sink(info.birthday.as_str()); // $ MISSING: sensitive=private - sink(info.emergency_contact.as_str()); // $ MISSING: sensitive=private - sink(info.name_of_employer.as_str()); // $ MISSING: sensitive=private + sink(info.my_ssn.as_str()); // $ sensitive=private + sink(&info.ssn); // $ sensitive=private + sink(info.ssn.data); // $ sensitive=private + sink(info.ssn.get_data()); // $ sensitive=private + sink(info.birthday.as_str()); // $ sensitive=private + sink(info.emergency_contact.as_str()); // $ sensitive=private + sink(info.name_of_employer.as_str()); // $ sensitive=private - sink(&info.medical_notes); // $ MISSING: sensitive=private - sink(info.medical_notes[0].as_str()); // $ MISSING: sensitive=private + sink(&info.gender); // $ MISSING: sensitive=private + sink(info.genderString.as_str()); // $ MISSING: sensitive=private + let sex = "Male"; + let gender = Gender::Female; + let a = Gender::Female; + sink(sex); // $ MISSING: sensitive=private + sink(gender); // $ MISSING: sensitive=private + sink(a); // $ MISSING: sensitive=private + + sink(info.patient_id); // $ MISSING: sensitive=private + sink(info.linkedPatientId); // $ MISSING: sensitive=private + sink(info.patient_record.as_str()); // $ MISSING: sensitive=private + sink(info.patient_record.trim()); // $ MISSING: sensitive=private + sink(&info.medical_notes); // $ sensitive=private + sink(info.medical_notes[0].as_str()); // $ sensitive=private for n in info.medical_notes.iter() { - sink(n.as_str()); // $ MISSING: sensitive=private + sink(n.as_str()); // $ sensitive=private } + sink(info.confidentialMessage.as_str()); // $ MISSING: sensitive=private + sink(info.confidentialMessage.to_lowercase()); // $ MISSING: sensitive=private - sink(info.latitude); // $ MISSING: sensitive=private + sink(info.latitude); // $ sensitive=private let x = info.longitude.unwrap(); - sink(x); // $ MISSING: sensitive=private + sink(x); // $ sensitive=private + + sink(info.financials.my_bank_account_number.as_str()); // $ sensitive=private SPURIOUS: sensitive=id + sink(info.financials.credit_card_no.as_str()); // $ sensitive=private + sink(info.financials.credit_rating); // $ sensitive=private + sink(info.financials.user_ccn.as_str()); // $ sensitive=private + sink(info.financials.cvv.as_str()); // $ MISSING: sensitive=private + sink(info.financials.beneficiary.as_str()); // $ MISSING: sensitive=private + sink(info.financials.routing_number); // $ MISSING: sensitive=private + sink(info.financials.routingNumberText.as_str()); // $ MISSING: sensitive=private + sink(info.financials.iban.as_str()); // $ MISSING: sensitive=private + sink(info.financials.iBAN.as_str()); // $ MISSING: sensitive=private + + sink(ContactDetails::HomePhoneNumber("123".to_string())); // $ sensitive=private + sink(ContactDetails::MobileNumber("123".to_string())); // $ sensitive=private + sink(ContactDetails::Email("a@b".to_string())); // $ MISSING: sensitive=private + + let numbers = [1, 2, 3]; + + if let ContactDetails::MobileNumber(num) = details { + sink(num.as_str()); // $ MISSING: sensitive=private + } + let contacts = numbers.map(|number| + { + let contact = ContactDetails::MobileNumber(number.to_string()); + sink(&contact); // $ sensitive=private + contact + } + ); + sink(&contacts[0]); // $ MISSING: sensitive=private + if let ContactDetails::HomePhoneNumber(num) = &contacts[0] { + sink(num.as_str()); // $ MISSING: sensitive=private + } - sink(info.financials.my_bank_account_number.as_str()); // $ MISSING: sensitive=private - sink(info.financials.credit_card_no.as_str()); // $ MISSING: sensitive=private - sink(info.financials.credit_rating); // $ MISSING: sensitive=private - sink(info.financials.user_ccn.as_str()); // $ MISSING: sensitive=private + let contacts2 = numbers.map(|number| + { + let contact = ContactDetails2 { + home_phone_number: number.to_string(), + }; + sink(&contact.home_phone_number); // $ sensitive=private + contact + } + ); + sink(&contacts2[0].home_phone_number); // $ sensitive=private // not private info + + let modulesEx = 1; + sink(modulesEx); + sink(info.financials.harmless.as_str()); + sink(info.financials.num_accounts); // $ SPURIOUS: sensitive=id + sink(info.financials.total_accounts); // $ SPURIOUS: sensitive=id + sink(info.financials.accounting); // $ SPURIOUS: sensitive=id + sink(info.financials.unaccounted); // $ SPURIOUS: sensitive=id + sink(info.financials.multiband); + + sink(ContactDetails::FavouriteColor("blue".to_string())); } diff --git a/rust/ql/test/query-tests/security/CWE-312/CleartextLogging.expected b/rust/ql/test/query-tests/security/CWE-312/CleartextLogging.expected index c21ce5b8aefa..594834bca1fe 100644 --- a/rust/ql/test/query-tests/security/CWE-312/CleartextLogging.expected +++ b/rust/ql/test/query-tests/security/CWE-312/CleartextLogging.expected @@ -28,6 +28,8 @@ | test_logging.rs:101:5:101:19 | ...::log | test_logging.rs:100:38:100:45 | password | test_logging.rs:101:5:101:19 | ...::log | This operation writes $@ to a log file. | test_logging.rs:100:38:100:45 | password | password | | test_logging.rs:119:5:119:42 | ...::log | test_logging.rs:119:28:119:41 | get_password(...) | test_logging.rs:119:5:119:42 | ...::log | This operation writes $@ to a log file. | test_logging.rs:119:28:119:41 | get_password(...) | get_password(...) | | test_logging.rs:132:5:132:32 | ...::log | test_logging.rs:130:25:130:32 | password | test_logging.rs:132:5:132:32 | ...::log | This operation writes $@ to a log file. | test_logging.rs:130:25:130:32 | password | password | +| test_logging.rs:139:5:139:38 | ...::log | test_logging.rs:139:27:139:37 | s1.password | test_logging.rs:139:5:139:38 | ...::log | This operation writes $@ to a log file. | test_logging.rs:139:27:139:37 | s1.password | s1.password | +| test_logging.rs:146:5:146:38 | ...::log | test_logging.rs:146:27:146:37 | s2.password | test_logging.rs:146:5:146:38 | ...::log | This operation writes $@ to a log file. | test_logging.rs:146:27:146:37 | s2.password | s2.password | | test_logging.rs:171:22:171:31 | log_expect | test_logging.rs:171:70:171:78 | password2 | test_logging.rs:171:22:171:31 | log_expect | This operation writes $@ to a log file. | test_logging.rs:171:70:171:78 | password2 | password2 | | test_logging.rs:175:24:175:33 | log_expect | test_logging.rs:175:72:175:80 | password2 | test_logging.rs:175:24:175:33 | log_expect | This operation writes $@ to a log file. | test_logging.rs:175:72:175:80 | password2 | password2 | | test_logging.rs:183:25:183:34 | log_unwrap | test_logging.rs:182:51:182:59 | password2 | test_logging.rs:183:25:183:34 | log_unwrap | This operation writes $@ to a log file. | test_logging.rs:182:51:182:59 | password2 | password2 | @@ -151,6 +153,10 @@ edges | test_logging.rs:132:12:132:31 | MacroExpr | test_logging.rs:132:5:132:32 | ...::log | provenance | MaD:12 Sink:MaD:12 | | test_logging.rs:132:28:132:29 | t1 [tuple.1] | test_logging.rs:132:28:132:31 | t1.1 | provenance | | | test_logging.rs:132:28:132:31 | t1.1 | test_logging.rs:132:12:132:31 | MacroExpr | provenance | | +| test_logging.rs:139:11:139:37 | MacroExpr | test_logging.rs:139:5:139:38 | ...::log | provenance | MaD:12 Sink:MaD:12 | +| test_logging.rs:139:27:139:37 | s1.password | test_logging.rs:139:11:139:37 | MacroExpr | provenance | | +| test_logging.rs:146:11:146:37 | MacroExpr | test_logging.rs:146:5:146:38 | ...::log | provenance | MaD:12 Sink:MaD:12 | +| test_logging.rs:146:27:146:37 | s2.password | test_logging.rs:146:11:146:37 | MacroExpr | provenance | | | test_logging.rs:171:33:171:79 | &... | test_logging.rs:171:22:171:31 | log_expect | provenance | MaD:9 Sink:MaD:9 | | test_logging.rs:171:33:171:79 | &... [&ref] | test_logging.rs:171:22:171:31 | log_expect | provenance | MaD:9 Sink:MaD:9 | | test_logging.rs:171:34:171:79 | MacroExpr | test_logging.rs:171:33:171:79 | &... | provenance | Config | @@ -382,6 +388,12 @@ nodes | test_logging.rs:132:12:132:31 | MacroExpr | semmle.label | MacroExpr | | test_logging.rs:132:28:132:29 | t1 [tuple.1] | semmle.label | t1 [tuple.1] | | test_logging.rs:132:28:132:31 | t1.1 | semmle.label | t1.1 | +| test_logging.rs:139:5:139:38 | ...::log | semmle.label | ...::log | +| test_logging.rs:139:11:139:37 | MacroExpr | semmle.label | MacroExpr | +| test_logging.rs:139:27:139:37 | s1.password | semmle.label | s1.password | +| test_logging.rs:146:5:146:38 | ...::log | semmle.label | ...::log | +| test_logging.rs:146:11:146:37 | MacroExpr | semmle.label | MacroExpr | +| test_logging.rs:146:27:146:37 | s2.password | semmle.label | s2.password | | test_logging.rs:171:22:171:31 | log_expect | semmle.label | log_expect | | test_logging.rs:171:33:171:79 | &... | semmle.label | &... | | test_logging.rs:171:33:171:79 | &... [&ref] | semmle.label | &... [&ref] | diff --git a/rust/ql/test/query-tests/security/CWE-312/test_logging.rs b/rust/ql/test/query-tests/security/CWE-312/test_logging.rs index d22453cbb3a4..3e8dbe816364 100644 --- a/rust/ql/test/query-tests/security/CWE-312/test_logging.rs +++ b/rust/ql/test/query-tests/security/CWE-312/test_logging.rs @@ -136,14 +136,14 @@ fn test_log(harmless: String, password: String, encrypted_password: String) { // logging from a struct let s1 = MyStruct1 { harmless: "foo".to_string(), password: "123456".to_string() }; // $ MISSING: Source=s1 warn!("message = {}", s1.harmless); - warn!("message = {}", s1.password); // $ MISSING: Alert[rust/cleartext-logging] + warn!("message = {}", s1.password); // $ Alert[rust/cleartext-logging] warn!("message = {}", s1); // $ MISSING: Alert[rust/cleartext-logging]=s1 warn!("message = {:?}", s1); // $ MISSING: Alert[rust/cleartext-logging]=s1 warn!("message = {:#?}", s1); // $ MISSING: Alert[rust/cleartext-logging]=s1 let s2 = MyStruct2 { harmless: "foo".to_string(), password: "123456".to_string() }; // $ MISSING: Source=s2 warn!("message = {}", s2.harmless); - warn!("message = {}", s2.password); // $ MISSING: Alert[rust/cleartext-logging] + warn!("message = {}", s2.password); // $ Alert[rust/cleartext-logging] warn!("message = {}", s2); // (this implementation does not output the password field) warn!("message = {:?}", s2); // $ MISSING: Alert[rust/cleartext-logging]=s2 warn!("message = {:#?}", s2); // $ MISSING: Alert[rust/cleartext-logging]=s2