Skip to content

Commit 294fd0a

Browse files
asgerfasgerf
authored
Merge pull request #18653 from asgerf/js/source-on-same-line
Test: Don't expect 'Source' tag when source and alert are on the same same
2 parents 3d3f07a + 7bf69d9 commit 294fd0a

File tree

3 files changed

+76
-68
lines changed

3 files changed

+76
-68
lines changed

‎javascript/ql/test/query-tests/Security/CWE-611/libxml.noent.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ express().post('/some/path', function (req, res) {
1313
// NOT OK: unguarded entity expansion
1414
libxmljs.parseXmlString(req.param("some-xml"), { noent: true }) // $ Alert
1515
// NOT OK: unguarded entity expansion
16-
libxmljs.parseXmlString(req.files.products.data.toString('utf8'), { noent: true })// $ Source=files $ Alert=files
16+
libxmljs.parseXmlString(req.files.products.data.toString('utf8'), { noent: true })// $ Alert
1717

1818
// OK - no entity expansion
1919
libxmljs.parseXmlString(req.files.products.data.toString('utf8'), { noent: false })

‎rust/ql/test/query-tests/security/CWE-312/test_logging.rs

+44-44
Original file line numberDiff line numberDiff line change
@@ -39,51 +39,51 @@ impl std::fmt::Display for MyStruct2 {
3939

4040
fn test_log(harmless: String, password: String, encrypted_password: String) {
4141
// logging macros
42-
debug!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
43-
error!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
44-
info!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
45-
trace!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
46-
warn!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
47-
log!(Level::Error, "message = {}", password); // $ Source Alert[rust/cleartext-logging]
42+
debug!("message = {}", password); // $ Alert[rust/cleartext-logging]
43+
error!("message = {}", password); // $ Alert[rust/cleartext-logging]
44+
info!("message = {}", password); // $ Alert[rust/cleartext-logging]
45+
trace!("message = {}", password); // $ Alert[rust/cleartext-logging]
46+
warn!("message = {}", password); // $ Alert[rust/cleartext-logging]
47+
log!(Level::Error, "message = {}", password); // $ Alert[rust/cleartext-logging]
4848

4949
// debug! macro, various formatting
5050
debug!("message");
5151
debug!("message = {}", harmless);
52-
debug!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
52+
debug!("message = {}", password); // $ Alert[rust/cleartext-logging]
5353
debug!("message = {}", encrypted_password);
54-
debug!("message = {} {}", harmless, password); // $ Source Alert[rust/cleartext-logging]
54+
debug!("message = {} {}", harmless, password); // $ Alert[rust/cleartext-logging]
5555
debug!("message = {harmless}");
56-
debug!("message = {harmless} {}", password); // $ Source Alert[rust/cleartext-logging]
57-
debug!("message = {password}"); // $ Source Alert[rust/cleartext-logging]
58-
debug!("message = {password:?}"); // $ Source Alert[rust/cleartext-logging]
56+
debug!("message = {harmless} {}", password); // $ Alert[rust/cleartext-logging]
57+
debug!("message = {password}"); // $ Alert[rust/cleartext-logging]
58+
debug!("message = {password:?}"); // $ Alert[rust/cleartext-logging]
5959
debug!(target: "target", "message = {}", harmless);
60-
debug!(target: "target", "message = {}", password); // $ Source Alert[rust/cleartext-logging]
61-
debug!(target: &password, "message = {}", harmless); // $ Source Alert[rust/cleartext-logging]
60+
debug!(target: "target", "message = {}", password); // $ Alert[rust/cleartext-logging]
61+
debug!(target: &password, "message = {}", harmless); // $ Alert[rust/cleartext-logging]
6262

6363
// log! macro, various formatting
6464
log!(Level::Error, "message = {}", harmless);
65-
log!(Level::Error, "message = {}", password); // $ Source Alert[rust/cleartext-logging]
65+
log!(Level::Error, "message = {}", password); // $ Alert[rust/cleartext-logging]
6666
log!(target: "target", Level::Error, "message = {}", harmless);
67-
log!(target: "target", Level::Error, "message = {}", password); // $ Source Alert[rust/cleartext-logging]
68-
log!(target: &password, Level::Error, "message = {}", harmless); // $ Source Alert[rust/cleartext-logging]
67+
log!(target: "target", Level::Error, "message = {}", password); // $ Alert[rust/cleartext-logging]
68+
log!(target: &password, Level::Error, "message = {}", harmless); // $ Alert[rust/cleartext-logging]
6969

7070
// structured logging
7171
error!(value = 1; "message = {}", harmless);
72-
error!(value = 1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
72+
error!(value = 1; "message = {}", password); // $ Alert[rust/cleartext-logging]
7373
error!(target: "target", value = 1; "message");
74-
error!(target: "target", value = 1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
75-
error!(target: &password, value = 1; "message"); // $ Source Alert[rust/cleartext-logging]
76-
error!(value = 1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
74+
error!(target: "target", value = 1; "message = {}", password); // $ Alert[rust/cleartext-logging]
75+
error!(target: &password, value = 1; "message"); // $ Alert[rust/cleartext-logging]
76+
error!(value = 1; "message = {}", password); // $ Alert[rust/cleartext-logging]
7777
error!(value = password.as_str(); "message"); // $ MISSING: Alert[rust/cleartext-logging]
7878
error!(value:? = password.as_str(); "message"); // $ MISSING: Alert[rust/cleartext-logging]
7979

8080
let value1 = 1;
8181
error!(value1; "message = {}", harmless);
82-
error!(value1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
82+
error!(value1; "message = {}", password); // $ Alert[rust/cleartext-logging]
8383
error!(target: "target", value1; "message");
84-
error!(target: "target", value1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
85-
error!(target: &password, value1; "message"); // $ Source Alert[rust/cleartext-logging]
86-
error!(value1; "message = {}", password); // $ Source Alert[rust/cleartext-logging]
84+
error!(target: "target", value1; "message = {}", password); // $ Alert[rust/cleartext-logging]
85+
error!(target: &password, value1; "message"); // $ Alert[rust/cleartext-logging]
86+
error!(value1; "message = {}", password); // $ Alert[rust/cleartext-logging]
8787

8888
let value2 = password.as_str();
8989
error!(value2; "message"); // $ MISSING: Alert[rust/cleartext-logging]
@@ -115,7 +115,7 @@ fn test_log(harmless: String, password: String, encrypted_password: String) {
115115
}
116116

117117
// logging with a call
118-
trace!("message = {}", get_password()); // $ Source Alert[rust/cleartext-logging]
118+
trace!("message = {}", get_password()); // $ Alert[rust/cleartext-logging]
119119

120120
let str1 = "123456".to_string();
121121
trace!("message = {}", &str1); // $ MISSING: Alert[rust/cleartext-logging]
@@ -149,36 +149,36 @@ fn test_log(harmless: String, password: String, encrypted_password: String) {
149149
}
150150

151151
fn test_std(password: String, i: i32, opt_i: Option<i32>) {
152-
print!("message = {}\n", password); // $ Source Alert[rust/cleartext-logging]
153-
println!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
154-
eprint!("message = {}\n", password); // $ Source Alert[rust/cleartext-logging]
155-
eprintln!("message = {}", password); // $ Source Alert[rust/cleartext-logging]
152+
print!("message = {}\n", password); // $ Alert[rust/cleartext-logging]
153+
println!("message = {}", password); // $ Alert[rust/cleartext-logging]
154+
eprint!("message = {}\n", password); // $ Alert[rust/cleartext-logging]
155+
eprintln!("message = {}", password); // $ Alert[rust/cleartext-logging]
156156

157157
match i {
158-
1 => { panic!("message = {}", password); } // $ Source Alert[rust/cleartext-logging]
159-
2 => { todo!("message = {}", password); } // $ Source Alert[rust/cleartext-logging]
160-
3 => { unimplemented!("message = {}", password); } // $ Source Alert[rust/cleartext-logging]
161-
4 => { unreachable!("message = {}", password); } // $ Source Alert[rust/cleartext-logging]
162-
5 => { assert!(false, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
163-
6 => { assert_eq!(1, 2, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
164-
7 => { assert_ne!(1, 1, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
165-
8 => { debug_assert!(false, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
166-
9 => { debug_assert_eq!(1, 2, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
167-
10 => { debug_assert_ne!(1, 1, "message = {}", password); } // $ Source Alert[rust/cleartext-logging]
168-
11 => { _ = opt_i.expect(format!("message = {}", password).as_str()); } // $ Source Alert[rust/cleartext-logging]
158+
1 => { panic!("message = {}", password); } // $ Alert[rust/cleartext-logging]
159+
2 => { todo!("message = {}", password); } // $ Alert[rust/cleartext-logging]
160+
3 => { unimplemented!("message = {}", password); } // $ Alert[rust/cleartext-logging]
161+
4 => { unreachable!("message = {}", password); } // $ Alert[rust/cleartext-logging]
162+
5 => { assert!(false, "message = {}", password); } // $ Alert[rust/cleartext-logging]
163+
6 => { assert_eq!(1, 2, "message = {}", password); } // $ Alert[rust/cleartext-logging]
164+
7 => { assert_ne!(1, 1, "message = {}", password); } // $ Alert[rust/cleartext-logging]
165+
8 => { debug_assert!(false, "message = {}", password); } // $ Alert[rust/cleartext-logging]
166+
9 => { debug_assert_eq!(1, 2, "message = {}", password); } // $ Alert[rust/cleartext-logging]
167+
10 => { debug_assert_ne!(1, 1, "message = {}", password); } // $ Alert[rust/cleartext-logging]
168+
11 => { _ = opt_i.expect(format!("message = {}", password).as_str()); } // $ Alert[rust/cleartext-logging]
169169
_ => {}
170170
}
171171

172172
std::io::stdout().lock().write_fmt(format_args!("message = {}\n", password)); // $ MISSING: Alert[rust/cleartext-logging]
173173
std::io::stderr().lock().write_fmt(format_args!("message = {}\n", password)); // $ MISSING: Alert[rust/cleartext-logging]
174-
std::io::stdout().lock().write(format!("message = {}\n", password).as_bytes()); // $ Source Alert[rust/cleartext-logging]
175-
std::io::stdout().lock().write_all(format!("message = {}\n", password).as_bytes()); // $ Source Alert[rust/cleartext-logging]
174+
std::io::stdout().lock().write(format!("message = {}\n", password).as_bytes()); // $ Alert[rust/cleartext-logging]
175+
std::io::stdout().lock().write_all(format!("message = {}\n", password).as_bytes()); // $ Alert[rust/cleartext-logging]
176176

177177
let mut out = std::io::stdout().lock();
178-
out.write(format!("message = {}\n", password).as_bytes()); // $ Source Alert[rust/cleartext-logging]
178+
out.write(format!("message = {}\n", password).as_bytes()); // $ Alert[rust/cleartext-logging]
179179

180180
let mut err = std::io::stderr().lock();
181-
err.write(format!("message = {}\n", password).as_bytes()); // $ Source Alert[rust/cleartext-logging]
181+
err.write(format!("message = {}\n", password).as_bytes()); // $ Alert[rust/cleartext-logging]
182182
}
183183

184184
fn main() {

‎shared/util/codeql/util/test/InlineExpectationsTest.qll

+31-23
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,21 @@ module TestPostProcessing {
645645
private import InlineExpectationsTest as InlineExpectationsTest
646646
private import InlineExpectationsTest::Make<Input>
647647

648+
/** Holds if the given locations refer to the same lines, but possibly with different column numbers. */
649+
bindingset[loc1, loc2]
650+
pragma[inline_late]
651+
private predicate sameLineInfo(Input::Location loc1, Input::Location loc2) {
652+
exists(string file, int line1, int line2 |
653+
loc1.hasLocationInfo(file, line1, _, line2, _) and
654+
loc2.hasLocationInfo(file, line1, _, line2, _)
655+
)
656+
}
657+
658+
pragma[nomagic]
659+
private predicate mainQueryResult(int row, int column, Input::Location loc) {
660+
queryResults(mainResultSet(), row, column, Input2::getRelativeUrl(loc))
661+
}
662+
648663
/**
649664
* Gets the tag to be used for the path-problem source at result row `row`.
650665
*
@@ -653,8 +668,10 @@ module TestPostProcessing {
653668
*/
654669
private string getSourceTag(int row) {
655670
getQueryKind() = "path-problem" and
656-
exists(string loc | queryResults(mainResultSet(), row, 2, loc) |
657-
if queryResults(mainResultSet(), row, 0, loc) then result = "Alert" else result = "Source"
671+
exists(Input::Location sourceLoc, Input::Location selectLoc |
672+
mainQueryResult(row, 0, selectLoc) and
673+
mainQueryResult(row, 2, sourceLoc) and
674+
if sameLineInfo(selectLoc, sourceLoc) then result = "Alert" else result = "Source"
658675
)
659676
}
660677

@@ -719,13 +736,10 @@ module TestPostProcessing {
719736
int row, Input::Location location, string element, string tag, string value
720737
) {
721738
getQueryKind() = "path-problem" and
722-
exists(string loc |
723-
queryResults(mainResultSet(), row, 2, loc) and
724-
queryResults(mainResultSet(), row, 3, element) and
725-
tag = getSourceTag(row) and
726-
value = "" and
727-
Input2::getRelativeUrl(location) = loc
728-
)
739+
mainQueryResult(row, 2, location) and
740+
queryResults(mainResultSet(), row, 3, element) and
741+
tag = getSourceTag(row) and
742+
value = ""
729743
}
730744

731745
predicate hasActualResult(Input::Location location, string element, string tag, string value) {
@@ -759,24 +773,18 @@ module TestPostProcessing {
759773
int row, Input::Location location, string element, string tag
760774
) {
761775
getQueryKind() = "path-problem" and
762-
exists(string loc |
763-
queryResults(mainResultSet(), row, 4, loc) and
764-
queryResults(mainResultSet(), row, 5, element) and
765-
tag = getSinkTag(row) and
766-
Input2::getRelativeUrl(location) = loc
767-
)
776+
mainQueryResult(row, 4, location) and
777+
queryResults(mainResultSet(), row, 5, element) and
778+
tag = getSinkTag(row)
768779
}
769780

770781
private predicate hasAlert(int row, Input::Location location, string element, string tag) {
771782
getQueryKind() = ["problem", "path-problem"] and
772-
exists(string loc |
773-
queryResults(mainResultSet(), row, 0, loc) and
774-
queryResults(mainResultSet(), row, 2, element) and
775-
tag = "Alert" and
776-
Input2::getRelativeUrl(location) = loc and
777-
not hasPathProblemSource(row, location, _, _, _) and
778-
not hasPathProblemSink(row, location, _, _)
779-
)
783+
mainQueryResult(row, 0, location) and
784+
queryResults(mainResultSet(), row, 2, element) and
785+
tag = "Alert" and
786+
not hasPathProblemSource(row, location, _, _, _) and
787+
not hasPathProblemSink(row, location, _, _)
780788
}
781789

782790
/**

0 commit comments

Comments
 (0)