diff --git a/CHANGELOG.md b/CHANGELOG.md index ec2f4d6..cb005cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,6 @@ +## v2.1.2 +### fix: Add sanitizer for redirects and clean sources +- fix: Add a sanitizer for `unvalidated-redirect` and clean sources ## v2.1.1 ### fix: Correct CI trigger and branch name; fix OWASP rules - fix: Fix sqli and crypto rules diff --git a/rules/java/lib/generic/servlet-untrusted-data-source.yaml b/rules/java/lib/generic/servlet-untrusted-data-source.yaml index f841a52..775a449 100644 --- a/rules/java/lib/generic/servlet-untrusted-data-source.yaml +++ b/rules/java/lib/generic/servlet-untrusted-data-source.yaml @@ -19,15 +19,11 @@ rules: - pattern: doDelete - pattern: doGet - pattern: doPost - - pattern: doDelete + - pattern: doPut - pattern: doTrace - pattern: _jspService - pattern: | $UNTRUSTED = (MessageBodyReader $READER).readFrom(...); - - pattern: | - (javax.servlet.http.Cookie[] $COOKIES) = (HttpServletRequest $REQ).getCookies(); - ... - $UNTRUSTED = $COOKIES[...].getValue(); - patterns: - pattern: | $FILES = ($FILE_UPLOAD_TYPE $SFU).parseRequest((HttpServletRequest $REQ)); diff --git a/rules/java/lib/generic/servlet-unvalidated-redirect-sinks.yaml b/rules/java/lib/generic/servlet-unvalidated-redirect-sinks.yaml index 28a8d34..87a544c 100644 --- a/rules/java/lib/generic/servlet-unvalidated-redirect-sinks.yaml +++ b/rules/java/lib/generic/servlet-unvalidated-redirect-sinks.yaml @@ -8,6 +8,11 @@ rules: provenance: https://gitlab.com/gitlab-org/security-products/sast-rules/-/blob/main/java/endpoint/rule-UnvalidatedRedirect.yml languages: - java - pattern-either: + mode: taint + pattern-sinks: - pattern: (HttpServletResponse $RES).sendRedirect($URL); - pattern: (HttpServletResponse $RES).addHeader("Location", $URL); + pattern-sanitizers: + - patterns: + - pattern: $URL = (HttpServletRequest $REQ).getContextPath(); + - focus-metavariable: $URL diff --git a/rules/java/lib/spring/untrusted-data-source.yaml b/rules/java/lib/spring/untrusted-data-source.yaml index bf5e7c9..66ac51c 100644 --- a/rules/java/lib/spring/untrusted-data-source.yaml +++ b/rules/java/lib/spring/untrusted-data-source.yaml @@ -29,12 +29,7 @@ rules: - pattern: PutMapping - pattern: | $UNTRUSTED = (MessageBodyReader $READER).readFrom(...); -# TODO: this will cause duplicate findings with servlet rules! - - pattern: | - (javax.servlet.http.Cookie[] $COOKIES) = (HttpServletRequest $REQ).getCookies(); - ... - $COOKIES[...].getValue(); - pattern: | Cookie $COOKIE = org.springframework.web.util.WebUtils.getCookie(...); ... - $COOKIE.getValue() + $UNTRUSTED = $COOKIE.getValue(); diff --git a/rules/java/lib/spring/untrusted-path-source.yaml b/rules/java/lib/spring/untrusted-path-source.yaml new file mode 100644 index 0000000..e0bc250 --- /dev/null +++ b/rules/java/lib/spring/untrusted-path-source.yaml @@ -0,0 +1,50 @@ +rules: + - id: spring-untrusted-path-source + options: + lib: true + severity: NOTE + message: Untrusted data flows from here + languages: + - java + patterns: + - pattern-either: + - patterns: + - pattern-either: + - pattern: | + @$ANNOTATION($URL) + $RETURNTYPE $METHODNAME(..., @PathVariable $TYPE $UNTRUSTED,...) { + ... + } + - patterns: + - pattern: | + @$ANNOTATION(...) + $RETURNTYPE $METHODNAME(..., $TYPE $UNTRUSTED,...) { + ... + } + - pattern-not: | + @$ANNOTATION(...) + $RETURNTYPE $METHODNAME(..., @PathVariable $TYPE $UNTRUSTED,...) { + ... + } + - metavariable-regex: + metavariable: $URL + regex: .*\*.* + - metavariable-regex: + metavariable: $TYPE + regex: ^(?!(Integer|Long|Float|Double|Char|Boolean|int|long|float|double|char|boolean|BindingResult|HttpMethod|java.util.TimeZone|java.util.Locale|java.util.ZoneId)) + - metavariable-pattern: + metavariable: $ANNOTATION + patterns: + - pattern-either: + - pattern: RequestMapping + - pattern: DeleteMapping + - pattern: GetMapping + - pattern: PatchMapping + - pattern: PostMapping + - pattern: PutMapping + - pattern: | + $UNTRUSTED = (MessageBodyReader $READER).readFrom(...); + - pattern: | + Cookie $COOKIE = org.springframework.web.util.WebUtils.getCookie(...); + ... + $UNTRUSTED = $COOKIE.getValue(); diff --git a/rules/java/security/path-traversal.yaml b/rules/java/security/path-traversal.yaml index 991c6a9..aaa2d81 100644 --- a/rules/java/security/path-traversal.yaml +++ b/rules/java/security/path-traversal.yaml @@ -265,7 +265,7 @@ rules: mode: join join: refs: - - rule: java/lib/spring/untrusted-data-source.yaml#spring-untrusted-data-source + - rule: java/lib/spring/untrusted-path-source.yaml#spring-untrusted-path-source as: untrusted-data - rule: java/lib/generic/path-traversal-sinks.yaml#java-path-traversal-sinks as: sink diff --git a/test/build.gradle.kts b/test/build.gradle.kts index baebb90..d257a52 100644 --- a/test/build.gradle.kts +++ b/test/build.gradle.kts @@ -62,6 +62,9 @@ dependencies { implementation("org.apache.commons:commons-email:1.6.0") implementation("org.apache.httpcomponents:httpclient:4.5.14") + // Apache Commons FileUpload for file upload samples + implementation("commons-fileupload:commons-fileupload:1.5") + // Apache Commons BeanUtils & Codec for bean/introspection & digest samples implementation("commons-beanutils:commons-beanutils:1.9.4") implementation("commons-codec:commons-codec:1.16.0") diff --git a/test/src/main/java/security/pathtraversal/PathTraversalServletSamples.java b/test/src/main/java/security/pathtraversal/PathTraversalServletSamples.java index 1dd109d..39331a1 100644 --- a/test/src/main/java/security/pathtraversal/PathTraversalServletSamples.java +++ b/test/src/main/java/security/pathtraversal/PathTraversalServletSamples.java @@ -7,6 +7,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.List; import javax.servlet.ServletException; import javax.servlet.annotation.WebServlet; @@ -15,6 +16,9 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.fileupload.FileItem; +import org.apache.commons.fileupload.servlet.ServletFileUpload; + import org.seqra.sast.test.util.NegativeRuleSample; import org.seqra.sast.test.util.PositiveRuleSample; @@ -251,6 +255,38 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) } } + /** + * VULNERABLE: uses filename from Apache Commons FileUpload directly in path construction. + */ + @WebServlet("/pathtraversal/unsafe-fileupload") + public static class UnsafeFileUploadServlet extends HttpServlet { + + private static final String UPLOAD_DIR = "/var/www/uploads/"; + + @Override + @PositiveRuleSample(value = "java/security/path-traversal.yaml", id = "path-traversal-in-servlet-app") + protected void doPost(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + + ServletFileUpload upload = new ServletFileUpload(); + try { + // Parse file upload request - untrusted source + List files = upload.parseRequest(request); + + for (FileItem file : files) { + String fileName = file.getName(); + + // Path traversal vulnerability: attacker can upload file with name like "../../etc/passwd" + File targetFile = new File(UPLOAD_DIR + fileName); + + streamFile(response, targetFile); + } + } catch (Exception e) { + response.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); + } + } + } + /** * Helper method to stream a java.io.File to the servlet response as octet-stream. */ diff --git a/test/src/main/java/security/pathtraversal/PathTraversalSpringSamples.java b/test/src/main/java/security/pathtraversal/PathTraversalSpringSamples.java index b208812..5d7316c 100644 --- a/test/src/main/java/security/pathtraversal/PathTraversalSpringSamples.java +++ b/test/src/main/java/security/pathtraversal/PathTraversalSpringSamples.java @@ -85,6 +85,26 @@ public ResponseEntity unsafeHeaderDownload(@RequestHeader("X- return streamPathUnchecked(path); } + + /** + * SAFE: @PathVariable without wildcard in URL pattern cannot contain slashes, + * so path traversal sequences like "../" are not possible. + */ + @GetMapping("/safe-pathvar/{fileName}") + @NegativeRuleSample(value = "java/security/path-traversal.yaml", id = "path-traversal-in-spring-app") + public ResponseEntity safeNonWildcardPathVariable(@PathVariable String fileName) { + + // Without a wildcard in the URL pattern (e.g., {*fileName}), + // the path variable cannot contain "/" characters, + // preventing path traversal attacks + Path path = Paths.get(BASE_DIR + fileName); + + if (!Files.exists(path) || !Files.isRegularFile(path)) { + return ResponseEntity.notFound().build(); + } + + return streamPathUnchecked(path); + } } @RestController diff --git a/test/src/main/java/security/unvalidatedredirect/UnvalidatedRedirectServletSamples.java b/test/src/main/java/security/unvalidatedredirect/UnvalidatedRedirectServletSamples.java index 3d39c6b..e34ba29 100644 --- a/test/src/main/java/security/unvalidatedredirect/UnvalidatedRedirectServletSamples.java +++ b/test/src/main/java/security/unvalidatedredirect/UnvalidatedRedirectServletSamples.java @@ -10,6 +10,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.seqra.sast.test.util.NegativeRuleSample; import org.seqra.sast.test.util.PositiveRuleSample; /** @@ -68,4 +69,20 @@ protected void doGet(HttpServletRequest request, HttpServletResponse response) } } } + + /** + * SAFE: redirect URL is from getContextPath() which is a sanitized source + * (not user-controlled, comes from server configuration). + */ + public static class SafeContextPathRedirectServlet extends HttpServlet { + + @Override + @NegativeRuleSample(value = "java/security/unvalidated-redirect.yaml", id = "unvalidated-redirect-in-servlet-app") + protected void doGet(HttpServletRequest request, HttpServletResponse response) + throws ServletException, IOException { + // SAFE: getContextPath() returns the server-configured context path, not user input + String url = request.getContextPath(); + response.sendRedirect(url + "/home.jsp"); + } + } }