Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions java/ql/src/experimental/CWE-532/SensitiveInfoLog.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
public static void main(String[] args) {
{
private static final Logger logger = LogManager.getLogger(SensitiveInfoLog.class);

String password = "Pass@0rd";

// BAD: user password is written to debug log
logger.debug("User password is "+password);
}

{
private static final Logger logger = LogManager.getLogger(SensitiveInfoLog.class);

String password = "Pass@0rd";

// GOOD: user password is never written to debug log
}
}
29 changes: 29 additions & 0 deletions java/ql/src/experimental/CWE-532/SensitiveInfoLog.qhelp
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>

<overview>
<p>Information written to log files can be of a sensitive nature and give valuable guidance to an attacker or expose sensitive user information. Third-party logging utilities like Log4J and SLF4J are widely used in Java projects. When sensitive information are written to logs without properly set logging levels, it is accessible to potential attackers who gains access to the
file storage.</p>
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
</overview>

<recommendation>
<p>Do not write secrets into the log files and enforce proper logging level control.</p>
</recommendation>

<example>
<p>The following example shows two ways of logging sensitive information. In the 'BAD' case,
the credentials are simply written to a debug log. In the 'GOOD' case, the credentials are never written to debug logs.</p>
<sample src="SensitiveInfoLog.java" />
</example>

<references>
<li>
<a href="https://cheatsheetseries.owasp.org/cheatsheets/Logging_Cheat_Sheet.html">OWASP Logging Guide</a>
</li>
<li>
<a href="https://cwe.mitre.org/data/definitions/532.html">CWE 532</a>
</li>
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
</references>
</qhelp>
83 changes: 83 additions & 0 deletions java/ql/src/experimental/CWE-532/SensitiveInfoLog.ql
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* @id java/sensitiveinfo-in-logfile
* @name Insertion of sensitive information into log files
* @description Writting sensitive information to log files can give valuable guidance to an attacker or expose sensitive user information.
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
* @kind problem
* @tags security
* external/cwe-532
*/

import java
import semmle.code.java.dataflow.TaintTracking
Comment thread
luchua-bc marked this conversation as resolved.
import DataFlow
import PathGraph
Comment thread
luchua-bc marked this conversation as resolved.

/** Class of popular logging utilities **/
class LoggerType extends RefType {
LoggerType() {
this.hasQualifiedName("org.apache.log4j", "Category") or //Log4J
this.hasQualifiedName("org.slf4j", "Logger") //SLF4j
}
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}

/** Concatenated string with a variable that keeps sensitive information judging by its name **/
class CredentialExpr extends Expr {
CredentialExpr() {
exists (Variable v | this.(AddExpr).getAnOperand() = v.getAnAccess() | v.getName().regexpMatch(getACredentialRegex()))
}
}

/** Source in concatenated string or variable itself **/
class CredentialSource extends DataFlow::ExprNode {
CredentialSource() {
exists (
Variable v | this.asExpr() = v.getAnAccess() | v.getName().regexpMatch(getACredentialRegex())
) or
exists (
this.asExpr().(AddExpr).getAnOperand().(CredentialExpr)
) or
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
exists (
this.asExpr().(CredentialExpr)
)
}
}

/**
* Gets a regular expression for matching names of variables that indicate the value being held is a credential.
*/

private string getACredentialRegex() {
result = "(?i).*pass(wd|word|code|phrase)(?!.*question).*" or
result = "(?i).*(uid|uuid|puid|username|userid|url).*"
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
}

class SensitiveLoggingSink extends DataFlow::ExprNode {
SensitiveLoggingSink() {
exists(MethodAccess ma |
ma.getMethod().getDeclaringType() instanceof LoggerType and
(
ma.getMethod().hasName("debug")
Comment thread
luchua-bc marked this conversation as resolved.
Outdated
) and
this.asExpr() = ma.getAnArgument()
)
}
}

class SensitiveLoggingConfig extends Configuration {
SensitiveLoggingConfig() {
this = "SensitiveLoggingConfig"
}

override predicate isSource(Node source) {
source instanceof CredentialSource
}

override predicate isSink(Node sink) {
sink instanceof SensitiveLoggingSink
}
}

from Node source, Node sink, SensitiveLoggingConfig conf, MethodAccess ma
where conf.hasFlow(source, sink) and ma.getAnArgument() = source.asExpr() and ma.getAnArgument() = sink.asExpr()
select "Outputting sensitive information $@ in method call $@.", source, ma, "to log files"
Comment thread
luchua-bc marked this conversation as resolved.
Outdated