From 2608d850e7ab46c3741531e0e020726cc7371d61 Mon Sep 17 00:00:00 2001 From: elsapet Date: Thu, 25 Jan 2024 15:53:51 +0200 Subject: [PATCH 1/4] feat(java): extend log injection rule --- rules/java/lang/log_injection.yml | 16 +++++++++++--- tests/java/lang/log_injection/test.js | 17 +++++++++++--- .../lang/log_injection/testdata/main.java | 22 +++++++++++++++++++ 3 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 tests/java/lang/log_injection/testdata/main.java diff --git a/rules/java/lang/log_injection.yml b/rules/java/lang/log_injection.yml index 33266ff17..3c4e57d2e 100644 --- a/rules/java/lang/log_injection.yml +++ b/rules/java/lang/log_injection.yml @@ -7,12 +7,22 @@ patterns: scope: result - variable: METHOD values: - - log + - config - debug - - warn - - info + - entering - error + - exiting + - fine + - finer + - finest + - info + - log + - logp + - logrb + - severe + - throwing - trace + - warn - variable: LOG values: - log diff --git a/tests/java/lang/log_injection/test.js b/tests/java/lang/log_injection/test.js index 3ea845e65..ec3a52104 100644 --- a/tests/java/lang/log_injection/test.js +++ b/tests/java/lang/log_injection/test.js @@ -1,13 +1,24 @@ -const { createInvoker, getEnvironment } = require("../../../helper.js") +const { createInvoker, createNewInvoker, getEnvironment } = require("../../../helper.js") const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) describe(ruleId, () => { const invoke = createInvoker(ruleId, ruleFile, testBase) - test("log_injection", () => { const testCase = "log_injection.java" expect(invoke(testCase)).toMatchSnapshot(); }) - + + // new style tests + const invokeV2 = createNewInvoker(ruleId, ruleFile, testBase) + + test("log_injection_v2", () => { + const testCase = "main.java" + + const results = invokeV2(testCase) + + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) + }) + }) \ No newline at end of file diff --git a/tests/java/lang/log_injection/testdata/main.java b/tests/java/lang/log_injection/testdata/main.java new file mode 100644 index 000000000..9143e8f6e --- /dev/null +++ b/tests/java/lang/log_injection/testdata/main.java @@ -0,0 +1,22 @@ +package inject; + +import javax.servlet.http.HttpServletRequest; +import java.util.logging.*; + +public class Log extends HttpServlet { + public void javaUtilLogging(HttpServletRequest req, HttpServletResponse res) { + String dangerous = req.getParameter("test"); + + logger = Logger.getLogger(Log.class); + logger.setLevel(Level.ALL); + + // bearer:expected java_lang_log_injection + logger.config(dangerous); + // bearer:expected java_lang_log_injection + logger.exiting("Exit:" + dangerous); + + // okay + logger.fine("hello world"); + } + +} From b143ada588c2bb611a00dbefeb5d5c4d931a596b Mon Sep 17 00:00:00 2001 From: elsapet Date: Thu, 25 Jan 2024 16:30:54 +0200 Subject: [PATCH 2/4] feat(java): add CRLF injection rule --- rules/java/lang/crlf_injection.yml | 110 ++++++++++++++++++ tests/java/lang/crlf_injection/test.js | 18 +++ .../lang/crlf_injection/testdata/main.java | 29 +++++ 3 files changed, 157 insertions(+) create mode 100644 rules/java/lang/crlf_injection.yml create mode 100644 tests/java/lang/crlf_injection/test.js create mode 100644 tests/java/lang/crlf_injection/testdata/main.java diff --git a/rules/java/lang/crlf_injection.yml b/rules/java/lang/crlf_injection.yml new file mode 100644 index 000000000..07791a3eb --- /dev/null +++ b/rules/java/lang/crlf_injection.yml @@ -0,0 +1,110 @@ +patterns: + - pattern: | + $.$($<...>$$<...>) + filters: + - variable: UNSANITIZED_USER_INPUT + detection: java_lang_log_dynamic_input + scope: result + - not: + variable: UNSANITIZED_USER_INPUT + detection: java_lang_log_sanitized_dynamic_input + scope: result + - not: + variable: UNSANITIZED_USER_INPUT + detection: java_lang_log_dynamic_bundle_input + scope: result + - variable: METHOD + values: + - config + - debug + - entering + - error + - exiting + - fine + - finer + - finest + - info + - log + - logp + - logrb + - severe + - throwing + - trace + - warn + - variable: LOG + values: + - log + - logger +auxiliary: + - id: java_lang_log_dynamic_bundle_input + patterns: + - pattern: $<_> + "bundle" + - id: java_lang_log_dynamic_input + patterns: + - pattern: $.$() + filters: + - variable: REQUEST + values: + - req + - request + - variable: REQUEST_METHOD + values: + - getCookies + - getHeader + - getQueryString + - getRequestURI + - getRequestURL + - getAttribute + - getInputStream + - getParameter + - getParameterMap + - getParameterNames + - getParameterValues + - getReader + - getHeaderNames + - getPart + - getParts + - id: java_lang_log_sanitized_dynamic_input + patterns: + - pattern: $<_>.$($, $<_>); + filters: + - variable: METHOD + values: + - replace + - replaceAll + - variable: SOURCE + string_regex: "\\r\\n|\\\\r\\\\n" + - pattern: $<_>.$($, $<_>).$($, $<_>); + filters: + - variable: METHOD + values: + - replace + - replaceAll + - variable: CR + string_regex: "\\r|\\\\r" + - variable: LF + string_regex: "\\n|\\\\n" +languages: + - java +metadata: + description: "Possible CLRF injection detected." + remediation_message: | + ## Description + + A CRLF (Carriage Return Line Feed) injection occurs when an attacker injects a sequence of line termination characters into a log message, allowing them to forge log entries. + + ## Remediations + + ✅ Strip any carriage return and line feed characters from user input data before logging it. + + ```java + logger.info(userInput.replaceAll("[\r\n]+", "")); + ``` + + ## Resources + - [OWASP CRLF Injection] (https://owasp.org/www-community/vulnerabilities/CRLF_Injection) + - [OWASP logging cheat sheet](https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html) + cwe_id: + - 93 + id: java_lang_crlf_injection + documentation_url: https://docs.bearer.com/reference/rules/java_lang_crlf_injection diff --git a/tests/java/lang/crlf_injection/test.js b/tests/java/lang/crlf_injection/test.js new file mode 100644 index 000000000..19ecb0824 --- /dev/null +++ b/tests/java/lang/crlf_injection/test.js @@ -0,0 +1,18 @@ +const { + createNewInvoker, + getEnvironment, +} = require("../../../helper.js") +const { ruleId, ruleFile, testBase } = getEnvironment(__dirname) + +describe(ruleId, () => { + const invoke = createNewInvoker(ruleId, ruleFile, testBase) + + test("crlf_injection", () => { + const testCase = "main.java" + + const results = invoke(testCase) + + expect(results.Missing).toEqual([]) + expect(results.Extra).toEqual([]) + }) +}) \ No newline at end of file diff --git a/tests/java/lang/crlf_injection/testdata/main.java b/tests/java/lang/crlf_injection/testdata/main.java new file mode 100644 index 000000000..79855fff7 --- /dev/null +++ b/tests/java/lang/crlf_injection/testdata/main.java @@ -0,0 +1,29 @@ +package inject; + +import javax.servlet.http.HttpServletRequest; +import java.util.logging.*; + +public class CRLFInjection extends HttpServlet { + public void javaUtilLogging(HttpServletRequest req, HttpServletResponse res) { + String dangerous = req.getParameter("test"); + String okay = "some known string"; + + logger = Logger.getLogger(Log.class); + logger.setLevel(Level.ALL); + + // bearer:expected java_lang_crlf_injection + logger.info(dangerous); + // bearer:expected java_lang_crlf_injection + logger.info(dangerous.replace("hello", "world")); + // bearer:expected java_lang_crlf_injection + logger.info(dangerous.replace('\n', "")); + // bearer:expected java_lang_crlf_injection + logger.info(dangerous.replaceAll("\r", "")); + + // okay + logger.config("hello world" + okay); + logger.info(dangerous.replace('\r', ' ').replace('\n', ' ')); + logger.logrb(Level.INFO, safe, safe, dangerous + "bundle", safe); + logger.fine(dangerous.replaceAll("[\r\n]+", "")); + } +} \ No newline at end of file From b7ee8dfc5da1303cdfb4ad9aa410a5a917686622 Mon Sep 17 00:00:00 2001 From: elsapet Date: Thu, 25 Jan 2024 17:00:11 +0200 Subject: [PATCH 3/4] refactor: clean up and add shared methods --- rules/java/lang/crlf_injection.yml | 58 ++++------------------- rules/java/lang/log_injection.yml | 50 ++----------------- rules/java/lang/logger.yml | 8 +--- rules/java/shared/lang/logger_methods.yml | 27 +++++++++++ 4 files changed, 44 insertions(+), 99 deletions(-) create mode 100644 rules/java/shared/lang/logger_methods.yml diff --git a/rules/java/lang/crlf_injection.yml b/rules/java/lang/crlf_injection.yml index 07791a3eb..aa94f39ff 100644 --- a/rules/java/lang/crlf_injection.yml +++ b/rules/java/lang/crlf_injection.yml @@ -1,9 +1,18 @@ +imports: + - java_shared_lang_user_input + - java_shared_lang_logger_methods patterns: - pattern: | $.$($<...>$$<...>) filters: + - variable: LOG + values: + - log + - logger + - variable: METHOD + detection: java_shared_lang_logger_methods - variable: UNSANITIZED_USER_INPUT - detection: java_lang_log_dynamic_input + detection: java_shared_lang_user_input scope: result - not: variable: UNSANITIZED_USER_INPUT @@ -13,57 +22,10 @@ patterns: variable: UNSANITIZED_USER_INPUT detection: java_lang_log_dynamic_bundle_input scope: result - - variable: METHOD - values: - - config - - debug - - entering - - error - - exiting - - fine - - finer - - finest - - info - - log - - logp - - logrb - - severe - - throwing - - trace - - warn - - variable: LOG - values: - - log - - logger auxiliary: - id: java_lang_log_dynamic_bundle_input patterns: - pattern: $<_> + "bundle" - - id: java_lang_log_dynamic_input - patterns: - - pattern: $.$() - filters: - - variable: REQUEST - values: - - req - - request - - variable: REQUEST_METHOD - values: - - getCookies - - getHeader - - getQueryString - - getRequestURI - - getRequestURL - - getAttribute - - getInputStream - - getParameter - - getParameterMap - - getParameterNames - - getParameterValues - - getReader - - getHeaderNames - - getPart - - getParts - id: java_lang_log_sanitized_dynamic_input patterns: - pattern: $<_>.$($, $<_>); diff --git a/rules/java/lang/log_injection.yml b/rules/java/lang/log_injection.yml index 3c4e57d2e..2fdf476bf 100644 --- a/rules/java/lang/log_injection.yml +++ b/rules/java/lang/log_injection.yml @@ -1,59 +1,19 @@ +imports: + - java_shared_lang_user_input + - java_shared_lang_logger_methods patterns: - pattern: | $.$($<...>$$<...>) filters: - variable: USER_INPUT - detection: java_lang_log_dynamic_input + detection: java_shared_lang_user_input scope: result - variable: METHOD - values: - - config - - debug - - entering - - error - - exiting - - fine - - finer - - finest - - info - - log - - logp - - logrb - - severe - - throwing - - trace - - warn + detection: java_shared_lang_logger_methods - variable: LOG values: - log - logger -auxiliary: - - id: java_lang_log_dynamic_input - patterns: - - pattern: | - $.$() - filters: - - variable: REQUEST - values: - - req - - request - - variable: REQUEST_METHOD - values: - - getCookies - - getHeader - - getQueryString - - getRequestURI - - getRequestURL - - getAttribute - - getInputStream - - getParameter - - getParameterMap - - getParameterNames - - getParameterValues - - getReader - - getHeaderNames - - getPart - - getParts languages: - java diff --git a/rules/java/lang/logger.yml b/rules/java/lang/logger.yml index e6cefbc29..20a8e1efb 100644 --- a/rules/java/lang/logger.yml +++ b/rules/java/lang/logger.yml @@ -1,5 +1,6 @@ imports: - java_shared_lang_datatype + - java_shared_lang_logger_methods patterns: - pattern: | $.$($<...>$$<...>) @@ -8,12 +9,7 @@ patterns: detection: java_shared_lang_datatype scope: result - variable: METHOD - values: - - log - - debug - - warn - - info - - error + detection: java_shared_lang_logger_methods - variable: LOG values: - log diff --git a/rules/java/shared/lang/logger_methods.yml b/rules/java/shared/lang/logger_methods.yml new file mode 100644 index 000000000..0e79ac98a --- /dev/null +++ b/rules/java/shared/lang/logger_methods.yml @@ -0,0 +1,27 @@ +type: shared +languages: + - java +patterns: + - pattern: $; + filters: + - variable: METHOD + values: + - config + - debug + - entering + - error + - exiting + - fine + - finer + - finest + - info + - log + - logp + - logrb + - severe + - throwing + - trace + - warn +metadata: + description: "Java Logger Methods" + id: java_shared_lang_logger_methods From 875b3ccd712e2a5950ac3303e3b8ba7018f90c25 Mon Sep 17 00:00:00 2001 From: elsapet Date: Mon, 29 Jan 2024 11:57:27 +0200 Subject: [PATCH 4/4] fix: improve bundle catch for logrb --- rules/java/lang/crlf_injection.yml | 33 ++++++++++++++++--- .../lang/crlf_injection/testdata/main.java | 9 +++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/rules/java/lang/crlf_injection.yml b/rules/java/lang/crlf_injection.yml index aa94f39ff..eb79a8a36 100644 --- a/rules/java/lang/crlf_injection.yml +++ b/rules/java/lang/crlf_injection.yml @@ -11,6 +11,10 @@ patterns: - logger - variable: METHOD detection: java_shared_lang_logger_methods + - not: + variable: METHOD + values: + - logrb - variable: UNSANITIZED_USER_INPUT detection: java_shared_lang_user_input scope: result @@ -18,14 +22,35 @@ patterns: variable: UNSANITIZED_USER_INPUT detection: java_lang_log_sanitized_dynamic_input scope: result + - pattern: | + $.logrb($<_>, $<_>, $$<...>) + filters: + - variable: LOG + values: + - log + - logger + - variable: UNSANITIZED_USER_INPUT + detection: java_shared_lang_user_input + scope: result - not: variable: UNSANITIZED_USER_INPUT - detection: java_lang_log_dynamic_bundle_input + detection: java_lang_log_sanitized_dynamic_input + scope: result + - pattern: | + $.logrb($<_>, $<_>, $<_>, $<_>, $$<...>) + filters: + - variable: LOG + values: + - log + - logger + - variable: UNSANITIZED_USER_INPUT + detection: java_shared_lang_user_input + scope: result + - not: + variable: UNSANITIZED_USER_INPUT + detection: java_lang_log_sanitized_dynamic_input scope: result auxiliary: - - id: java_lang_log_dynamic_bundle_input - patterns: - - pattern: $<_> + "bundle" - id: java_lang_log_sanitized_dynamic_input patterns: - pattern: $<_>.$($, $<_>); diff --git a/tests/java/lang/crlf_injection/testdata/main.java b/tests/java/lang/crlf_injection/testdata/main.java index 79855fff7..ed672589b 100644 --- a/tests/java/lang/crlf_injection/testdata/main.java +++ b/tests/java/lang/crlf_injection/testdata/main.java @@ -20,6 +20,15 @@ public void javaUtilLogging(HttpServletRequest req, HttpServletResponse res) { // bearer:expected java_lang_crlf_injection logger.info(dangerous.replaceAll("\r", "")); + // logrb cases + // - logrb​(Level level, ResourceBundle bundle, String msg, Object... params) + // - logrb​(Level level, String sourceClass, String sourceMethod, ResourceBundle bundle, String msg, Object... params) + + // bearer:expected java_lang_crlf_injection + logger.logrb(Level.INFO, safe, dangerous, safe, safe); + // bearer:expected java_lang_crlf_injection + logger.logrb(Level.INFO, safe, safe, ResourceBundle.getBundle("package.ExampleResource", locale), dangerous, safe); + // okay logger.config("hello world" + okay); logger.info(dangerous.replace('\r', ' ').replace('\n', ' '));