Skip to content

Commit 679eb0d

Browse files
committed
Add failing test for @pattern validation
1 parent 6a8204d commit 679eb0d

File tree

7 files changed

+815
-275
lines changed

7 files changed

+815
-275
lines changed

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

Lines changed: 474 additions & 273 deletions
Large diffs are not rendered by default.

java/ql/test/query-tests/security/CWE-918/SanitizationTests.java

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22
import java.net.URI;
33
import java.net.http.HttpClient;
44
import java.net.http.HttpRequest;
5-
import java.util.regex.Pattern;
65
import java.util.regex.Matcher;
6+
import java.util.regex.Pattern;
7+
import java.util.List;
78

89
import javax.servlet.ServletException;
910
import javax.servlet.http.HttpServlet;
@@ -147,6 +148,32 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response)
147148
HttpRequest r13 = HttpRequest.newBuilder(new URI(param13)).build();
148149
client.send(r13, null);
149150
}
151+
152+
// GOOD: sanitisation by @Pattern annotation on a field
153+
AnnotatedFieldObject obj14 = new AnnotatedFieldObject(request.getParameter("uri14"));
154+
HttpRequest r14a = HttpRequest.newBuilder(new URI(obj14.uri)).build();
155+
client.send(r14a, null);
156+
HttpRequest r14b = HttpRequest.newBuilder(new URI(obj14.getUri())).build();
157+
client.send(r14b, null);
158+
159+
// GOOD: sanitisation by @Pattern annotation on a parameter of a constructor
160+
AnnotatedParameterObject obj15 = new AnnotatedParameterObject(request.getParameter("uri15"));
161+
HttpRequest r15a = HttpRequest.newBuilder(new URI(obj15.uri)).build();
162+
client.send(r15a, null);
163+
HttpRequest r15b = HttpRequest.newBuilder(new URI(obj15.getUri())).build();
164+
client.send(r15b, null);
165+
166+
// GOOD: sanitisation by @Pattern annotation on a parameter of a method
167+
HttpRequest r16 = HttpRequest.newBuilder(new URI(identity1(request.getParameter("uri16")))).build();
168+
client.send(r16, null);
169+
170+
// GOOD: sanitisation by @Pattern annotation on a method (which constrains the return value)
171+
HttpRequest r17 = HttpRequest.newBuilder(new URI(identity2(request.getParameter("uri17")))).build();
172+
client.send(r17, null);
173+
174+
// GOOD: sanitisation by @Pattern annotation on a type (we do not recognise this, so we get an FP)
175+
HttpRequest r18 = HttpRequest.newBuilder(new URI(getFromList(List.of(request.getParameter("uri18"))))).build(); // $ SPURIOUS: Source Alert
176+
client.send(r18, null); // $ SPURIOUS: Alert
150177
} catch (Exception e) {
151178
// TODO: handle exception
152179
}
@@ -157,4 +184,44 @@ private void validate(String s) {
157184
throw new IllegalArgumentException("Invalid ID");
158185
}
159186
}
187+
188+
public String identity1(@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9_-]+") String uri) {
189+
return uri;
190+
}
191+
192+
@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9_-]+")
193+
public String identity2(String uri) {
194+
return uri;
195+
}
196+
197+
public String getFromList(List<@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9_-]+") String> list) {
198+
return list.get(0);
199+
}
200+
201+
public class AnnotatedFieldObject {
202+
@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9_-]+")
203+
String uri;
204+
205+
String otherField;
206+
207+
public AnnotatedFieldObject(String uri) {
208+
this.uri = uri;
209+
}
210+
211+
public String getUri() {
212+
return uri;
213+
}
214+
}
215+
216+
public class AnnotatedParameterObject {
217+
String uri;
218+
219+
public AnnotatedParameterObject(@javax.validation.constraints.Pattern(regexp = "[a-zA-Z0-9_-]+") String uri) {
220+
this.uri = uri;
221+
}
222+
223+
public String getUri() {
224+
return uri;
225+
}
226+
}
160227
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
//semmle-extractor-options: --javac-args -source 11 -target 11 -cp ${testdir}/../../../stubs/springframework-5.8.x:${testdir}/../../../stubs/javax-ws-rs-api-2.1.1:${testdir}/../../../stubs/javax-ws-rs-api-3.0.0:${testdir}/../../../stubs/apache-http-4.4.13/:${testdir}/../../../stubs/projectreactor-3.4.3/:${testdir}/../../../stubs/postgresql-42.3.3/:${testdir}/../../../stubs/HikariCP-3.4.5/:${testdir}/../../../stubs/spring-jdbc-5.3.8/:${testdir}/../../../stubs/jdbi3-core-3.27.2/:${testdir}/../../../stubs/cargo:${testdir}/../../../stubs/javafx-web:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/stapler-1.263:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/apache-http-5:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jaxws-api-2.0:${testdir}/../../../stubs/apache-cxf
1+
//semmle-extractor-options: --javac-args -source 11 -target 11 -cp ${testdir}/../../../stubs/javax-validation-constraints:${testdir}/../../../stubs/springframework-5.8.x:${testdir}/../../../stubs/javax-ws-rs-api-2.1.1:${testdir}/../../../stubs/javax-ws-rs-api-3.0.0:${testdir}/../../../stubs/apache-http-4.4.13/:${testdir}/../../../stubs/projectreactor-3.4.3/:${testdir}/../../../stubs/postgresql-42.3.3/:${testdir}/../../../stubs/HikariCP-3.4.5/:${testdir}/../../../stubs/spring-jdbc-5.3.8/:${testdir}/../../../stubs/jdbi3-core-3.27.2/:${testdir}/../../../stubs/cargo:${testdir}/../../../stubs/javafx-web:${testdir}/../../../stubs/apache-commons-jelly-1.0.1:${testdir}/../../../stubs/dom4j-2.1.1:${testdir}/../../../stubs/jaxen-1.2.0:${testdir}/../../../stubs/stapler-1.263:${testdir}/../../../stubs/javax-servlet-2.5:${testdir}/../../../stubs/apache-commons-fileupload-1.4:${testdir}/../../../stubs/saxon-xqj-9.x:${testdir}/../../../stubs/apache-commons-beanutils:${testdir}/../../../stubs/apache-commons-lang:${testdir}/../../../stubs/apache-http-5:${testdir}/../../../stubs/playframework-2.6.x:${testdir}/../../../stubs/jaxws-api-2.0:${testdir}/../../../stubs/apache-cxf
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<project>
2+
<modelVersion>4.0.0</modelVersion>
3+
<groupId>com.example</groupId>
4+
<artifactId>stub-generation</artifactId>
5+
<version>1.0-SNAPSHOT</version>
6+
<dependencies>
7+
<dependency>
8+
<groupId>javax.validation</groupId>
9+
<artifactId>validation-api</artifactId>
10+
<version>2.0.1.Final</version>
11+
</dependency>
12+
</dependencies>
13+
</project>

java/ql/test/stubs/javax-validation-constraints/javax/validation/Constraint.java

Lines changed: 88 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

java/ql/test/stubs/javax-validation-constraints/javax/validation/Payload.java

Lines changed: 23 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

java/ql/test/stubs/javax-validation-constraints/javax/validation/constraints/Pattern.java

Lines changed: 148 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)