From 42a84a18b01fd2860c5c4594328cb33688f3142d Mon Sep 17 00:00:00 2001 From: Porcupiney Hairs Date: Sun, 12 Jul 2020 03:45:15 +0530 Subject: [PATCH 1/3] JAVA : Add query to detect Apache Structs enabled DEvmode This query detects cases where the development mode is enabled for a struts configuration. I can't find a CVE per se but, at present, [Github's fuzzy search](https://github.com/search?q=%3Cconstant+name%3D%22struts.devMode%22+value%3D%22true%22+%2F%3E+language%3Axml&type=Code) returns more than 44000 results. Some of them look like they are classroom projects, so they may be ineligible for a CVE. But we should be flagging them anyways as setting the development on in a production system is a very bad practice and can often lead to remote code execution. So these should be fixed anyways. --- .../Security/CWE/CWE-489/StrutsBad.xml | 11 +++++ .../Security/CWE/CWE-489/StrutsGood.xml | 11 +++++ .../Security/CWE/CWE-489/devMode.qhelp | 32 +++++++++++++++ .../Security/CWE/CWE-489/devMode.ql | 24 +++++++++++ .../semmle/code/xml/StrutsXML.qll | 41 +++++++++++++++++++ .../security/CWE-489/StrutsBad.xml | 11 +++++ .../security/CWE-489/StrutsGood.xml | 11 +++++ .../security/CWE-489/devMode.expected | 1 + .../security/CWE-489/devMode.qlref | 1 + 9 files changed, 143 insertions(+) create mode 100644 java/ql/src/experimental/Security/CWE/CWE-489/StrutsBad.xml create mode 100644 java/ql/src/experimental/Security/CWE/CWE-489/StrutsGood.xml create mode 100644 java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp create mode 100644 java/ql/src/experimental/Security/CWE/CWE-489/devMode.ql create mode 100644 java/ql/src/experimental/semmle/code/xml/StrutsXML.qll create mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml create mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml create mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected create mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/StrutsBad.xml b/java/ql/src/experimental/Security/CWE/CWE-489/StrutsBad.xml new file mode 100644 index 00000000000..64e614cc525 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-489/StrutsBad.xml @@ -0,0 +1,11 @@ + + + + + + + + + diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/StrutsGood.xml b/java/ql/src/experimental/Security/CWE/CWE-489/StrutsGood.xml new file mode 100644 index 00000000000..369b42a7ec9 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-489/StrutsGood.xml @@ -0,0 +1,11 @@ + + + + + + + + + \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp new file mode 100644 index 00000000000..14d9b6311b8 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp @@ -0,0 +1,32 @@ + + + + +

Turning Apache Struts' development mode configuration on while deploying applications to production environments can lead to remote code execution.

+ +
+ + +

An application should disable the development mode at the time of deployment.

+ +
+ + +

The following example shows a `struts.xml` file with `struts.devmode` enabled.

+ + + +

This can be easily corrected by setting the value of the `struts.devmode` parameter to false.

+ + + +
+ + +
  • + Apache Struts: + Struts development mode configuration +
  • + +
    +
    diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/devMode.ql b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.ql new file mode 100644 index 00000000000..1bbb1b71ab4 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.ql @@ -0,0 +1,24 @@ +/** + * @name Apache Struts development mode enabled + * @description Enabling struts development mode in production environment + * can lead to remote code execution. + * @kind problem + * @problem.severity error + * @precision high + * @id java/struts-development-mode + * @tags security + * external/cwe/cwe-489 + */ + +import java +import experimental.semmle.code.xml.StrutsXML + +bindingset[path] +predicate isLikelyDemoProject(string path) { path.regexpMatch("(?i).*(demo|test|example).*") } + +from ConstantParameter c +where + c.getNameValue() = "struts.devMode" and + c.getValueValue() = "true" and + not isLikelyDemoProject(c.getFile().getRelativePath()) +select c, "Enabling development mode in production environments is dangerous" diff --git a/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll b/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll new file mode 100644 index 00000000000..bda7824560c --- /dev/null +++ b/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll @@ -0,0 +1,41 @@ +import java + +/** + * A deployment descriptor file, typically called `struts.xml`. + */ +class StrutsXMLFile extends XMLFile { + StrutsXMLFile() { + count(XMLElement e | e = this.getAChild()) = 1 and + this.getAChild().getName() = "struts" + } +} + +/** + * An XML element in a `StrutsXMLFile`. + */ +class StrutsXMLElement extends XMLElement { + StrutsXMLElement() { this.getFile() instanceof StrutsXMLFile } + + /** + * Gets the value for this element, with leading and trailing whitespace trimmed. + */ + string getValue() { result = allCharactersString().trim() } +} + +/** + * A `` element in a `StrutsXMLFile`. + */ +class ConstantParameter extends StrutsXMLElement { + ConstantParameter() { this.getName() = "constant" } + + /** + * Gets the value of the `name` attribute of this ``. + */ + string getNameValue() { result = getAttributeValue("name") } + + /** + * Gets the value of the `value` attribute of this ``. + */ + string getValueValue() { result = getAttributeValue("value") } + +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml new file mode 100644 index 00000000000..4ee9c1647cf --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml @@ -0,0 +1,11 @@ + + + + + + + + + \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml new file mode 100644 index 00000000000..369b42a7ec9 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml @@ -0,0 +1,11 @@ + + + + + + + + + \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected new file mode 100644 index 00000000000..fec3ff3c2c8 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected @@ -0,0 +1 @@ +| StrutsBad.xml:8:5:8:52 | constant | Enabling development mode in production environments is dangerous | diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref new file mode 100644 index 00000000000..da70fb9a6c4 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref @@ -0,0 +1 @@ +experimental/Security/CWE/CWE-489/devMode.ql From 5151a528ac6d7b4b54543a6c36bbc49d32a0e62b Mon Sep 17 00:00:00 2001 From: Porcuiney Hairs Date: Mon, 1 Mar 2021 22:59:30 +0530 Subject: [PATCH 2/3] Include suggestions from review --- java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp | 4 ++-- java/ql/src/experimental/semmle/code/xml/StrutsXML.qll | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp index 14d9b6311b8..4775bd8e0e2 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp +++ b/java/ql/src/experimental/Security/CWE/CWE-489/devMode.qhelp @@ -14,11 +14,11 @@

    The following example shows a `struts.xml` file with `struts.devmode` enabled.

    - +

    This can be easily corrected by setting the value of the `struts.devmode` parameter to false.

    - + diff --git a/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll b/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll index bda7824560c..06e1dab79af 100644 --- a/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll +++ b/java/ql/src/experimental/semmle/code/xml/StrutsXML.qll @@ -37,5 +37,4 @@ class ConstantParameter extends StrutsXMLElement { * Gets the value of the `value` attribute of this ``. */ string getValueValue() { result = getAttributeValue("value") } - } From beb15e27eb21077d1ec915be0ec39303d1a8fe2b Mon Sep 17 00:00:00 2001 From: Porcuiney Hairs Date: Tue, 2 Mar 2021 18:13:33 +0530 Subject: [PATCH 3/3] remove tests --- .../query-tests/security/CWE-489/StrutsBad.xml | 11 ----------- .../query-tests/security/CWE-489/StrutsGood.xml | 11 ----------- .../query-tests/security/CWE-489/devMode.expected | 1 - .../query-tests/security/CWE-489/devMode.qlref | 1 - 4 files changed, 24 deletions(-) delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml deleted file mode 100644 index 4ee9c1647cf..00000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsBad.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml b/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml deleted file mode 100644 index 369b42a7ec9..00000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-489/StrutsGood.xml +++ /dev/null @@ -1,11 +0,0 @@ - - - - - - - - - \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected deleted file mode 100644 index fec3ff3c2c8..00000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.expected +++ /dev/null @@ -1 +0,0 @@ -| StrutsBad.xml:8:5:8:52 | constant | Enabling development mode in production environments is dangerous | diff --git a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref b/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref deleted file mode 100644 index da70fb9a6c4..00000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-489/devMode.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-489/devMode.ql