Skip to content

Commit 77904d9

Browse files
smowtonsmowton
committed
Remove failing test
The case where something might be exactly a constant is general across all queries, and not handled yet, particularly in the case where the result of `getParameter("uri")` might have changed between the check and the use.
1 parent 6933d06 commit 77904d9

File tree

2 files changed

+28
-33
lines changed

2 files changed

+28
-33
lines changed

‎java/ql/test/query-tests/security/CWE-918/RequestForgery.expected

Lines changed: 28 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,13 @@ edges
1515
| RequestForgery.java:19:23:19:58 | new URI(...) : URI | RequestForgery.java:27:57:27:59 | uri |
1616
| RequestForgery.java:19:31:19:57 | getParameter(...) : String | RequestForgery.java:19:23:19:58 | new URI(...) : URI |
1717
| RequestForgery.java:19:31:19:57 | getParameter(...) : String | RequestForgery.java:22:52:22:54 | uri |
18-
| RequestForgery.java:19:31:19:57 | getParameter(...) : String | RequestForgery.java:27:57:27:59 | uri |
19-
| RequestForgery.java:61:33:61:63 | getParameter(...) : String | RequestForgery.java:62:59:62:77 | new URI(...) |
20-
| RequestForgery.java:65:49:65:79 | getParameter(...) : String | RequestForgery.java:66:59:66:77 | new URI(...) |
21-
| RequestForgery.java:70:31:70:61 | getParameter(...) : String | RequestForgery.java:71:59:71:88 | new URI(...) |
22-
| RequestForgery.java:74:73:74:103 | getParameter(...) : String | RequestForgery.java:75:59:75:77 | new URI(...) |
23-
| RequestForgery.java:78:56:78:86 | getParameter(...) : String | RequestForgery.java:79:59:79:77 | new URI(...) |
24-
| RequestForgery.java:82:55:82:85 | getParameter(...) : String | RequestForgery.java:83:59:83:77 | new URI(...) |
18+
| RequestForgery.java:59:33:59:63 | getParameter(...) : String | RequestForgery.java:60:59:60:77 | new URI(...) |
19+
| RequestForgery.java:63:49:63:79 | getParameter(...) : String | RequestForgery.java:64:59:64:77 | new URI(...) |
20+
| RequestForgery.java:68:31:68:61 | getParameter(...) : String | RequestForgery.java:69:59:69:88 | new URI(...) |
21+
| RequestForgery.java:72:73:72:103 | getParameter(...) : String | RequestForgery.java:73:59:73:77 | new URI(...) |
22+
| RequestForgery.java:76:56:76:86 | getParameter(...) : String | RequestForgery.java:77:59:77:77 | new URI(...) |
23+
| RequestForgery.java:80:55:80:85 | getParameter(...) : String | RequestForgery.java:81:59:81:77 | new URI(...) |
24+
| RequestForgery.java:84:33:84:63 | getParameter(...) : String | RequestForgery.java:85:59:85:77 | new URI(...) |
2525
| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:32:47:32:67 | ... + ... |
2626
| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:37:43:37:56 | fooResourceUrl |
2727
| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:41:42:41:55 | fooResourceUrl |
@@ -49,19 +49,20 @@ nodes
4949
| RequestForgery.java:19:23:19:58 | new URI(...) : URI | semmle.label | new URI(...) : URI |
5050
| RequestForgery.java:19:31:19:57 | getParameter(...) : String | semmle.label | getParameter(...) : String |
5151
| RequestForgery.java:22:52:22:54 | uri | semmle.label | uri |
52-
| RequestForgery.java:27:57:27:59 | uri | semmle.label | uri |
53-
| RequestForgery.java:61:33:61:63 | getParameter(...) : String | semmle.label | getParameter(...) : String |
54-
| RequestForgery.java:62:59:62:77 | new URI(...) | semmle.label | new URI(...) |
55-
| RequestForgery.java:65:49:65:79 | getParameter(...) : String | semmle.label | getParameter(...) : String |
56-
| RequestForgery.java:66:59:66:77 | new URI(...) | semmle.label | new URI(...) |
57-
| RequestForgery.java:70:31:70:61 | getParameter(...) : String | semmle.label | getParameter(...) : String |
58-
| RequestForgery.java:71:59:71:88 | new URI(...) | semmle.label | new URI(...) |
59-
| RequestForgery.java:74:73:74:103 | getParameter(...) : String | semmle.label | getParameter(...) : String |
60-
| RequestForgery.java:75:59:75:77 | new URI(...) | semmle.label | new URI(...) |
61-
| RequestForgery.java:78:56:78:86 | getParameter(...) : String | semmle.label | getParameter(...) : String |
62-
| RequestForgery.java:79:59:79:77 | new URI(...) | semmle.label | new URI(...) |
63-
| RequestForgery.java:82:55:82:85 | getParameter(...) : String | semmle.label | getParameter(...) : String |
64-
| RequestForgery.java:83:59:83:77 | new URI(...) | semmle.label | new URI(...) |
52+
| RequestForgery.java:59:33:59:63 | getParameter(...) : String | semmle.label | getParameter(...) : String |
53+
| RequestForgery.java:60:59:60:77 | new URI(...) | semmle.label | new URI(...) |
54+
| RequestForgery.java:63:49:63:79 | getParameter(...) : String | semmle.label | getParameter(...) : String |
55+
| RequestForgery.java:64:59:64:77 | new URI(...) | semmle.label | new URI(...) |
56+
| RequestForgery.java:68:31:68:61 | getParameter(...) : String | semmle.label | getParameter(...) : String |
57+
| RequestForgery.java:69:59:69:88 | new URI(...) | semmle.label | new URI(...) |
58+
| RequestForgery.java:72:73:72:103 | getParameter(...) : String | semmle.label | getParameter(...) : String |
59+
| RequestForgery.java:73:59:73:77 | new URI(...) | semmle.label | new URI(...) |
60+
| RequestForgery.java:76:56:76:86 | getParameter(...) : String | semmle.label | getParameter(...) : String |
61+
| RequestForgery.java:77:59:77:77 | new URI(...) | semmle.label | new URI(...) |
62+
| RequestForgery.java:80:55:80:85 | getParameter(...) : String | semmle.label | getParameter(...) : String |
63+
| RequestForgery.java:81:59:81:77 | new URI(...) | semmle.label | new URI(...) |
64+
| RequestForgery.java:84:33:84:63 | getParameter(...) : String | semmle.label | getParameter(...) : String |
65+
| RequestForgery.java:85:59:85:77 | new URI(...) | semmle.label | new URI(...) |
6566
| SpringSSRF.java:26:33:26:60 | getParameter(...) : String | semmle.label | getParameter(...) : String |
6667
| SpringSSRF.java:32:47:32:67 | ... + ... | semmle.label | ... + ... |
6768
| SpringSSRF.java:37:43:37:56 | fooResourceUrl | semmle.label | fooResourceUrl |
@@ -83,13 +84,13 @@ nodes
8384
| RequestForgery2.java:67:43:67:45 | uri | RequestForgery2.java:23:27:23:53 | getParameter(...) : String | RequestForgery2.java:67:43:67:45 | uri | Potential server side request forgery due to $@. | RequestForgery2.java:23:27:23:53 | getParameter(...) | a user-provided value |
8485
| RequestForgery2.java:69:29:69:32 | uri2 | RequestForgery2.java:23:27:23:53 | getParameter(...) : String | RequestForgery2.java:69:29:69:32 | uri2 | Potential server side request forgery due to $@. | RequestForgery2.java:23:27:23:53 | getParameter(...) | a user-provided value |
8586
| RequestForgery.java:22:52:22:54 | uri | RequestForgery.java:19:31:19:57 | getParameter(...) : String | RequestForgery.java:22:52:22:54 | uri | Potential server side request forgery due to $@. | RequestForgery.java:19:31:19:57 | getParameter(...) | a user-provided value |
86-
| RequestForgery.java:27:57:27:59 | uri | RequestForgery.java:19:31:19:57 | getParameter(...) : String | RequestForgery.java:27:57:27:59 | uri | Potential server side request forgery due to $@. | RequestForgery.java:19:31:19:57 | getParameter(...) | a user-provided value |
87-
| RequestForgery.java:62:59:62:77 | new URI(...) | RequestForgery.java:61:33:61:63 | getParameter(...) : String | RequestForgery.java:62:59:62:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:61:33:61:63 | getParameter(...) | a user-provided value |
88-
| RequestForgery.java:66:59:66:77 | new URI(...) | RequestForgery.java:65:49:65:79 | getParameter(...) : String | RequestForgery.java:66:59:66:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:65:49:65:79 | getParameter(...) | a user-provided value |
89-
| RequestForgery.java:71:59:71:88 | new URI(...) | RequestForgery.java:70:31:70:61 | getParameter(...) : String | RequestForgery.java:71:59:71:88 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:70:31:70:61 | getParameter(...) | a user-provided value |
90-
| RequestForgery.java:75:59:75:77 | new URI(...) | RequestForgery.java:74:73:74:103 | getParameter(...) : String | RequestForgery.java:75:59:75:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:74:73:74:103 | getParameter(...) | a user-provided value |
91-
| RequestForgery.java:79:59:79:77 | new URI(...) | RequestForgery.java:78:56:78:86 | getParameter(...) : String | RequestForgery.java:79:59:79:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:78:56:78:86 | getParameter(...) | a user-provided value |
92-
| RequestForgery.java:83:59:83:77 | new URI(...) | RequestForgery.java:82:55:82:85 | getParameter(...) : String | RequestForgery.java:83:59:83:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:82:55:82:85 | getParameter(...) | a user-provided value |
87+
| RequestForgery.java:60:59:60:77 | new URI(...) | RequestForgery.java:59:33:59:63 | getParameter(...) : String | RequestForgery.java:60:59:60:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:59:33:59:63 | getParameter(...) | a user-provided value |
88+
| RequestForgery.java:64:59:64:77 | new URI(...) | RequestForgery.java:63:49:63:79 | getParameter(...) : String | RequestForgery.java:64:59:64:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:63:49:63:79 | getParameter(...) | a user-provided value |
89+
| RequestForgery.java:69:59:69:88 | new URI(...) | RequestForgery.java:68:31:68:61 | getParameter(...) : String | RequestForgery.java:69:59:69:88 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:68:31:68:61 | getParameter(...) | a user-provided value |
90+
| RequestForgery.java:73:59:73:77 | new URI(...) | RequestForgery.java:72:73:72:103 | getParameter(...) : String | RequestForgery.java:73:59:73:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:72:73:72:103 | getParameter(...) | a user-provided value |
91+
| RequestForgery.java:77:59:77:77 | new URI(...) | RequestForgery.java:76:56:76:86 | getParameter(...) : String | RequestForgery.java:77:59:77:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:76:56:76:86 | getParameter(...) | a user-provided value |
92+
| RequestForgery.java:81:59:81:77 | new URI(...) | RequestForgery.java:80:55:80:85 | getParameter(...) : String | RequestForgery.java:81:59:81:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:80:55:80:85 | getParameter(...) | a user-provided value |
93+
| RequestForgery.java:85:59:85:77 | new URI(...) | RequestForgery.java:84:33:84:63 | getParameter(...) : String | RequestForgery.java:85:59:85:77 | new URI(...) | Potential server side request forgery due to $@. | RequestForgery.java:84:33:84:63 | getParameter(...) | a user-provided value |
9394
| SpringSSRF.java:32:47:32:67 | ... + ... | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:32:47:32:67 | ... + ... | Potential server side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value |
9495
| SpringSSRF.java:37:43:37:56 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:37:43:37:56 | fooResourceUrl | Potential server side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value |
9596
| SpringSSRF.java:41:42:41:55 | fooResourceUrl | SpringSSRF.java:26:33:26:60 | getParameter(...) : String | SpringSSRF.java:41:42:41:55 | fooResourceUrl | Potential server side request forgery due to $@. | SpringSSRF.java:26:33:26:60 | getParameter(...) | a user-provided value |

‎java/ql/test/query-tests/security/CWE-918/RequestForgery.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
2222
HttpRequest r = HttpRequest.newBuilder(uri).build();
2323
client.send(r, null);
2424

25-
// GOOD: the request parameter is validated against a known fixed string
26-
if (VALID_URI.equals(request.getParameter("uri"))) {
27-
HttpRequest r2 = HttpRequest.newBuilder(uri).build();
28-
client.send(r2, null);
29-
}
30-
3125
// GOOD: sanitisation by concatenation with a prefix that prevents targeting an arbitrary host.
3226
// We test a few different ways of sanitisation: via string conctentation (perhaps nested),
3327
// via a stringbuilder and via String.format.

0 commit comments

Comments
 (0)