Minor corrections in QLDoc, qhelp and example code

github · atorralba · Jan 11, 2022 · Oct 29, 2021 · Oct 29, 2021 · Nov 2, 2021
commit 474bf576a7c73ddd2a976e3d54049aa423f15f5c
@@ -8,7 +8,7 @@ import semmle.code.java.security.LogInjection
  * A taint-tracking configuration for tracking untrusted user input used in log entries.
  */
 class LogInjectionConfiguration extends TaintTracking::Configuration {
-  LogInjectionConfiguration() { this = "Log Injection" }
+  LogInjectionConfiguration() { this = "LogInjectionConfiguration" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
 

@@ -29,16 +29,15 @@ other forms of HTML injection.
 </recommendation>
 
 <example>
-  <p>In the example, a username, provided by the user, is logged using <code>logger.warn</code> (from  <code>org.slf4j.Logger</code>). 
+  <p>In the first example, a username, provided by the user, is logged using <code>logger.warn</code> (from  <code>org.slf4j.Logger</code>). 
     In the first case (<code>/bad</code> endpoint), the username is logged without any sanitization.
     If a malicious user provides <code>Guest'%0AUser:'Admin</code> as a username parameter, 
     the log entry will be split into two separate lines, where the first line will be <code>User:'Guest'</code> and the second one will be <code>User:'Admin'</code>.
 </p>
 <sample src="LogInjectionBad.java" />
 
-<p> In the second case (<code>/good</code> endpoint), <code>matches()</code> is used to ensure the user input only has alphanumeric characters.
-  If a malicious user provides `Guest'%0AUser:'Admin` as a username parameter, 
-  the log entry will not be split into two separate lines, resulting in a single line <code>User:'Guest'User:'Admin'</code>.</p>
+<p> In the second example (<code>/good</code> endpoint), <code>matches()</code> is used to ensure the user input only has alphanumeric characters.
+  If a malicious user provides `Guest'%0AUser:'Admin` as a username parameter, the log entry will not be logged at all, preventing the injection.</p>
 
 <sample src="LogInjectionGood.java" />
 </example>

@@ -1,9 +1,10 @@
 /**
  * @name Log Injection
- * @description Building log entries from user-controlled data is vulnerable to
- *              insertion of forged log entries by a malicious user.
+ * @description Building log entries from user-controlled data may allow
+ *              insertion of forged log entries by malicious users.
  * @kind path-problem
  * @problem.severity error
+ * @security-severity 7.8
  * @precision high
  * @id java/log-injection
  * @tags security

@@ -16,9 +16,9 @@ public class LogInjection {
     public String good(@RequestParam(value = "username", defaultValue = "name") String username) {
         // The regex check here, allows only alphanumeric characters to pass.
         // Hence, does not result in log injection
-        if (username.matches("\w*")) {
+        if (username.matches("\\w*")) {
             log.warn("User:'{}'", username);
-            
+
             return username;
         }
     }