hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-24 08:15:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Now alerts about exposing exception.getMessage() in servlet responses are split out of java/stack-trace-exposure into its own alert java/error-message-exposure because this is a better fit.

Browse Source
This commit is contained in:
Daniel Winther Petersen
2024-07-25 18:02:54 +02:00
parent 90b25a6696
commit 1c1ba7734f
12 changed files with 128 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
java/ql/lib/semmle/code/java/dataflow/ApiSources.qll
View File

@@ -19,5 +19,6 @@ private module AllApiSources {
private import semmle.code.java.security.InsecureTrustManager
private import semmle.code.java.security.JWT
private import semmle.code.java.security.StackTraceExposureQuery
private import semmle.code.java.security.SensitiveDataExposureThroughErrorMessageQuery
private import semmle.code.java.security.ZipSlipQuery
}

34
java/ql/lib/semmle/code/java/security/SensitiveDataExposureThroughErrorMessageQuery.qll Normal file
View File

@@ -0,0 +1,34 @@
/** Provides predicates to reason about exposure of error messages. */
import java
private import semmle.code.java.dataflow.FlowSources
private import semmle.code.java.security.InformationLeak
/**
* A get message source node.
*/
private class GetMessageFlowSource extends ApiSourceNode {
GetMessageFlowSource() {
exists(Method method | this.asExpr().(MethodCall).getMethod() = method |
method.hasName("getMessage") and
method.hasNoParameters() and
method.getDeclaringType().hasQualifiedName("java.lang", "Throwable")
)
}
}
private module GetMessageFlowSourceToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof GetMessageFlowSource }
predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
}
private module GetMessageFlowSourceToHttpResponseSinkFlow =
TaintTracking::Global<GetMessageFlowSourceToHttpResponseSinkFlowConfig>;
/**
* Holds if there is a call to `getMessage()` that then flows to a servlet response.
*/
predicate getMessageFlowsExternally(DataFlow::Node externalExpr, GetMessageFlowSource getMessage) {
GetMessageFlowSourceToHttpResponseSinkFlow::flow(getMessage, externalExpr)
}

29
java/ql/lib/semmle/code/java/security/StackTraceExposureQuery.qll
View File

@@ -93,32 +93,3 @@ predicate stringifiedStackFlowsExternally(DataFlow::Node externalExpr, Expr stac
StackTraceStringToHttpResponseSinkFlow::flow(DataFlow::exprNode(stackTraceString), externalExpr)
)
}
/**
* A get message source node.
*/
private class GetMessageFlowSource extends ApiSourceNode {
GetMessageFlowSource() {
exists(Method method | this.asExpr().(MethodCall).getMethod() = method |
method.hasName("getMessage") and
method.hasNoParameters() and
method.getDeclaringType().hasQualifiedName("java.lang", "Throwable")
)
}
}
private module GetMessageFlowSourceToHttpResponseSinkFlowConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node src) { src instanceof GetMessageFlowSource }
predicate isSink(DataFlow::Node sink) { sink instanceof InformationLeakSink }
}
private module GetMessageFlowSourceToHttpResponseSinkFlow =
TaintTracking::Global<GetMessageFlowSourceToHttpResponseSinkFlowConfig>;
/**
* Holds if there is a call to `getMessage()` that then flows to a servlet response.
*/
predicate getMessageFlowsExternally(DataFlow::Node externalExpr, GetMessageFlowSource getMessage) {
GetMessageFlowSourceToHttpResponseSinkFlow::flow(getMessage, externalExpr)
}

22
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.java Normal file
View File

@@ -0,0 +1,22 @@
protected void doGet(HttpServletRequest request, HttpServletResponse response) {
try {
doSomeWork();
} catch (NullPointerException ex) {
// BAD: printing a exception message back to the response
response.sendError(
HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
ex.getMessage());
return;
}
try {
doSomeWork();
} catch (NullPointerException ex) {
// GOOD: log the stack trace, and send back a non-revealing response
log("Exception occurred", ex.getMessage);
response.sendError(
HttpServletResponse.SC_INTERNAL_SERVER_ERROR,
"Exception occurred");
return;
}
}

41
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.qhelp Normal file
View File

@@ -0,0 +1,41 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>The error message at the top of a stack trace can include
information such as server-side file names and SQL code that the
application relies on, allowing an attacker to fine-tune a subsequent
injection attack.</p>
</overview>
<recommendation>
<p>Send the user a more generic error message that reveals less information.
Either suppress the error message entirely, or log it only on the server.</p>
</recommendation>
<example>
<p>In the following example, an exception is handled in two different
ways. In the first version, labeled BAD, the exception is sent back to
the remote user using the <code>getMessage()</code> method. As such,
the user is able to see a detailed error message, which may contain
sensitive information. In the second version, the error message is
logged only on the server. That way, the developers can still access
and use the error log, but remote users will not see the
information.</p>
<sample src="SensitiveDataExposureThroughErrorMessage.java" />
</example>
<references>
<li>OWASP: <a href="https://owasp.org/www-community/Improper_Error_Handling">Improper Error Handling</a>.</li>
<li>CERT Java Coding Standard:
<a href="https://www.securecoding.cert.org/confluence/display/java/ERR01-J.+Do+not+allow+exceptions+to+expose+sensitive+information">ERR01-J.
Do not allow exceptions to expose sensitive information</a>.</li>
<li>
CWE-209: <a href="https://cwe.mitre.org/data/definitions/209.html">Information Exposure Through an Error Message</a>.
</li>
</references>
</qhelp>

22
java/ql/src/Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.ql Normal file
View File

@@ -0,0 +1,22 @@
/**
* @name Information exposure through a error message
* @description Information from a error message propagates to an external user.
* Error messages can unintentionally reveal implementation details
* that are useful to an attacker for developing a subsequent exploit.
* @kind problem
* @problem.severity error
* @security-severity 5.4
* @precision high
* @id java/error-message-exposure
* @tags security
* external/cwe/cwe-209
*/
import java
import semmle.code.java.dataflow.DataFlow
import semmle.code.java.security.SensitiveDataExposureThroughErrorMessageQuery
from Expr externalExpr, Expr errorInformation
where
getMessageFlowsExternally(DataFlow::exprNode(externalExpr), DataFlow::exprNode(errorInformation))
select externalExpr, "$@ can be exposed to an external user.", errorInformation, "Error information"

6
java/ql/src/Security/CWE/CWE-209/StackTraceExposure.qhelp
View File

@@ -12,11 +12,7 @@ final state of the software when the error occurred.</p>
<p>Unfortunately, the same information can be useful to an attacker.
The sequence of class names in a stack trace can reveal the structure
of the application as well as any internal components it relies on.
Furthermore, the error message at the top of a stack trace can include
information such as server-side file names and SQL code that the
application relies on, allowing an attacker to fine-tune a subsequent
injection attack.</p>
of the application as well as any internal components it relies on.</p>
</overview>
<recommendation>

3
java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql
View File

@@ -20,6 +20,5 @@ import semmle.code.java.security.StackTraceExposureQuery
from Expr externalExpr, Expr errorInformation
where
printsStackExternally(externalExpr, errorInformation) or
stringifiedStackFlowsExternally(DataFlow::exprNode(externalExpr), errorInformation) or
getMessageFlowsExternally(DataFlow::exprNode(externalExpr), DataFlow::exprNode(errorInformation))
stringifiedStackFlowsExternally(DataFlow::exprNode(externalExpr), errorInformation)
select externalExpr, "$@ can be exposed to an external user.", errorInformation, "Error information"

4
java/ql/src/change-notes/2024-07-25-java-error-message-exposure.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Now alerts about exposing `exception.getMessage()` in servlet responses are split out of `java/stack-trace-exposure` into its own alert `java/error-message-exposure` because this is a better fit.

1
java/ql/test/query-tests/security/CWE-209/semmle/tests/SensitiveDataExposureThroughErrorMessage.expected Normal file
View File

@@ -0,0 +1 @@
| Test.java:55:5:55:19 | getMessage(...) | $@ can be exposed to an external user. | Test.java:55:5:55:19 | getMessage(...) | Error information |

1
java/ql/test/query-tests/security/CWE-209/semmle/tests/SensitiveDataExposureThroughErrorMessage.qlref Normal file
View File

@@ -0,0 +1 @@
Security/CWE/CWE-209/SensitiveDataExposureThroughErrorMessage.ql

1
java/ql/test/query-tests/security/CWE-209/semmle/tests/StackTraceExposure.expected
View File

@@ -1,4 +1,3 @@
| Test.java:25:4:25:43 | printStackTrace(...) | $@ can be exposed to an external user. | Test.java:25:4:25:5 | ex | Error information |
| Test.java:35:5:35:18 | printTrace(...) | $@ can be exposed to an external user. | Test.java:65:3:65:4 | ex | Error information |
| Test.java:45:5:45:19 | printTrace2(...) | $@ can be exposed to an external user. | Test.java:71:3:71:4 | ex | Error information |
| Test.java:55:5:55:19 | getMessage(...) | $@ can be exposed to an external user. | Test.java:55:5:55:19 | getMessage(...) | Error information |