Merge pull request #6612 from Marcono1234/marcono1234/literal-getLiteral-usage

Java: Replace incorrect usage of `Literal.getLiteral()`

Author: aschackmull

java/ql/lib/semmle/code/java/Annotation.qll

@@ -118,11 +118,7 @@ class Annotatable extends Element {
    * annotation attached to it for the specified `category`.
    */
   predicate suppressesWarningsAbout(string category) {
-    exists(string withQuotes |
-      withQuotes = getAnAnnotation().(SuppressWarningsAnnotation).getASuppressedWarning()
-    |
-      category = withQuotes.substring(1, withQuotes.length() - 1)
-    )
+    category = getAnAnnotation().(SuppressWarningsAnnotation).getASuppressedWarning()
     or
     this.(Member).getDeclaringType().suppressesWarningsAbout(category)
     or

java/ql/lib/semmle/code/java/Expr.qll

@@ -599,10 +599,23 @@ class AssignURShiftExpr extends AssignOp, @assignurshiftexpr {
 
 /** A common super-class to represent constant literals. */
 class Literal extends Expr, @literal {
-  /** Gets a string representation of this literal. */
+  /**
+   * Gets a string representation of this literal as it appeared
+   * in the source code.
+   *
+   * **Important:** Unless a query explicitly wants to check how
+   * a literal was written in the source code, the predicate
+   * `getValue()` (or value predicates of subclasses) should be
+   * used instead. For example for the integer literal `0x7fff_ffff`
+   * the result of `getLiteral()` would be `0x7fff_ffff`, while
+   * the result of `getValue()` would be `2147483647`.
+   */
   string getLiteral() { namestrings(result, _, this) }
 
-  /** Gets a string representation of the value of this literal. */
+  /**
+   * Gets a string representation of the value this literal
+   * represents.
+   */
   string getValue() { namestrings(_, result, this) }
 
   /** Gets a printable representation of this expression. */
@@ -619,9 +632,9 @@ class Literal extends Expr, @literal {
 class BooleanLiteral extends Literal, @booleanliteral {
   /** Gets the boolean representation of this literal. */
   boolean getBooleanValue() {
-    result = true and getLiteral() = "true"
+    result = true and getValue() = "true"
     or
-    result = false and getLiteral() = "false"
+    result = false and getValue() = "false"
   }
 
   override string getAPrimaryQlClass() { result = "BooleanLiteral" }

java/ql/lib/semmle/code/java/JDKAnnotations.qll

@@ -25,10 +25,7 @@ class SuppressWarningsAnnotation extends Annotation {
   }
 
   /** Gets the name of a warning suppressed by this annotation. */
-  string getASuppressedWarning() {
-    result = this.getAValue().(StringLiteral).getLiteral() or
-    result = this.getAValue().(ArrayInit).getAnInit().(StringLiteral).getLiteral()
-  }
+  string getASuppressedWarning() { result = getASuppressedWarningLiteral().getRepresentedString() }
 }
 
 /** A `@Target` annotation. */

java/ql/lib/semmle/code/java/security/Encryption.qll

@@ -168,7 +168,7 @@ string getInsecureAlgorithmRegex() {
 string getASecureAlgorithmName() {
   result =
     [
-      "RSA", "SHA256", "SHA512", "CCM", "GCM", "AES([^a-zA-Z](?!ECB|CBC/PKCS[57]Padding)).*",
+      "RSA", "SHA256", "SHA512", "CCM", "GCM", "AES(?![^a-zA-Z](ECB|CBC/PKCS[57]Padding))",
       "Blowfish", "ECIES"
     ]
 }

java/ql/lib/semmle/code/java/security/RelativePaths.qll

@@ -5,7 +5,7 @@ import java
  * An element that starts with a relative path.
  */
 predicate relativePath(Element tree, string command) {
-  exists(StringLiteral lit, string text | tree = lit and text = lit.getLiteral() |
+  exists(StringLiteral lit, string text | tree = lit and text = lit.getRepresentedString() |
     text != "" and
     (
       text.regexpMatch("[^/\\\\ \t]*") or

java/ql/src/Likely Bugs/Collections/WriteOnlyContainer.ql

@@ -24,7 +24,7 @@ where
   // Exclude fields that may be read from reflectively.
   not reflectivelyRead(v) and
   // Exclude fields annotated with `@SuppressWarnings("unused")`.
-  not v.getAnAnnotation().(SuppressWarningsAnnotation).getASuppressedWarning() = "\"unused\"" and
+  not v.getAnAnnotation().(SuppressWarningsAnnotation).getASuppressedWarning() = "unused" and
   // Exclude fields with relevant Lombok annotations.
   not v instanceof LombokGetterAnnotatedField and
   // Every access to `v` is either...

java/ql/src/Security/CWE/CWE-327/BrokenCryptoAlgorithm.ql

@@ -17,14 +17,14 @@ import DataFlow
 import PathGraph
 
 private class ShortStringLiteral extends StringLiteral {
-  ShortStringLiteral() { getLiteral().length() < 100 }
+  ShortStringLiteral() { getRepresentedString().length() < 100 }
 }
 
 class BrokenAlgoLiteral extends ShortStringLiteral {
   BrokenAlgoLiteral() {
-    getValue().regexpMatch(getInsecureAlgorithmRegex()) and
+    getRepresentedString().regexpMatch(getInsecureAlgorithmRegex()) and
     // Exclude German and French sentences.
-    not getValue().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
+    not getRepresentedString().regexpMatch(".*\\p{IsLowercase} des \\p{IsLetter}.*")
   }
 }
 
@@ -48,4 +48,4 @@ where
   source.getNode().asExpr() = s and
   conf.hasFlowPath(source, sink)
 select c, source, sink, "Cryptographic algorithm $@ is weak and should not be used.", s,
-  s.getLiteral()
+  s.getRepresentedString()

java/ql/src/Security/CWE/CWE-327/MaybeBrokenCryptoAlgorithm.ql

@@ -18,14 +18,14 @@ import semmle.code.java.dispatch.VirtualDispatch
 import PathGraph
 
 private class ShortStringLiteral extends StringLiteral {
-  ShortStringLiteral() { getLiteral().length() < 100 }
+  ShortStringLiteral() { getRepresentedString().length() < 100 }
 }
 
 class InsecureAlgoLiteral extends ShortStringLiteral {
   InsecureAlgoLiteral() {
     // Algorithm identifiers should be at least two characters.
-    getValue().length() > 1 and
-    exists(string s | s = getLiteral() |
+    getRepresentedString().length() > 1 and
+    exists(string s | s = getRepresentedString() |
       not s.regexpMatch(getSecureAlgorithmRegex()) and
       // Exclude results covered by another query.
       not s.regexpMatch(getInsecureAlgorithmRegex())
@@ -72,4 +72,4 @@ where
   conf.hasFlowPath(source, sink)
 select c, source, sink,
   "Cryptographic algorithm $@ may not be secure, consider using a different algorithm.", s,
-  s.getLiteral()
+  s.getRepresentedString()

java/ql/src/Violations of Best Practice/Magic Constants/MagicConstants.qll

@@ -186,24 +186,21 @@ private predicate trivialIntValue(string s) {
   exists(string pos | trivialPositiveIntValue(pos) and s = "-" + pos)
 }
 
-private predicate intTrivial(Literal lit) {
-  exists(string v | trivialIntValue(v) and v = lit.getLiteral())
+private predicate intTrivial(IntegerLiteral lit) {
+  // Remove all `_` from literal, if any (e.g. `1_000_000`)
+  exists(string v | trivialIntValue(v) and v = lit.getLiteral().replaceAll("_", ""))
 }
 
-private predicate longTrivial(Literal lit) {
-  exists(string v | trivialIntValue(v) and v + "L" = lit.getLiteral())
+private predicate longTrivial(LongLiteral lit) {
+  exists(string v |
+    trivialIntValue(v) and
+    // Remove all `_` from literal, if any (e.g. `1_000_000L`)
+    v + ["l", "L"] = lit.getLiteral().replaceAll("_", "")
+  )
 }
 
 private predicate powerOfTen(float f) {
-  f = 10 or
-  f = 100 or
-  f = 1000 or
-  f = 10000 or
-  f = 100000 or
-  f = 1000000 or
-  f = 10000000 or
-  f = 100000000 or
-  f = 1000000000
+  f = [10, 100, 1000, 10000, 100000, 1000000, 10000000, 100000000, 1000000000]
 }
 
 private predicate floatTrivial(Literal lit) {
@@ -244,7 +241,7 @@ private predicate literalIsConstantInitializer(Literal literal, Field f) {
 }
 
 private predicate nonTrivialValue(string value, Literal literal, string context) {
-  value = literal.getLiteral() and
+  value = literal.getValue() and
   not trivial(literal) and
   not literalIsConstantInitializer(literal, _) and
   not literal.getParent*() instanceof ArrayInit and
@@ -259,7 +256,7 @@ private predicate valueOccurrenceCount(string value, int n, string context) {
 
 private predicate occurenceCount(Literal lit, string value, int n, string context) {
   valueOccurrenceCount(value, n, context) and
-  value = lit.getLiteral() and
+  value = lit.getValue() and
   nonTrivialValue(_, lit, context)
 }
 
@@ -296,14 +293,7 @@ private predicate firstOccurrence(Literal lit, string value, string context, int
   )
 }
 
-predicate isNumber(Literal lit) {
-  lit.getType().getName() = "char" or
-  lit.getType().getName() = "short" or
-  lit.getType().getName() = "int" or
-  lit.getType().getName() = "long" or
-  lit.getType().getName() = "float" or
-  lit.getType().getName() = "double"
-}
+predicate isNumber(Literal lit) { lit.getType() instanceof NumericOrCharType }
 
 predicate magicConstant(Literal e, string msg) {
   exists(string value, int n, string context |
@@ -320,7 +310,7 @@ predicate magicConstant(Literal e, string msg) {
 
 private predicate relevantField(Field f, string value) {
   exists(Literal lit |
-    not trivial(lit) and value = lit.getLiteral() and literalIsConstantInitializer(lit, f)
+    not trivial(lit) and value = lit.getValue() and literalIsConstantInitializer(lit, f)
   )
 }
 
@@ -344,7 +334,7 @@ private predicate candidateConstantForLiteral(
   exists(Literal initLiteral |
     literalIsConstantInitializer(initLiteral, constField) and
     exists(string value |
-      value = initLiteral.getLiteral() and
+      value = initLiteral.getValue() and
       nonTrivialValue(value, magicLiteral, context) and
       fieldUsedInContext(constField, context)
     ) and

java/ql/test/library-tests/Encryption/insecure.ql

@@ -2,5 +2,5 @@ import default
 import semmle.code.java.security.Encryption
 
 from StringLiteral s
-where s.getLiteral().regexpMatch(getInsecureAlgorithmRegex())
+where s.getRepresentedString().regexpMatch(getInsecureAlgorithmRegex())
 select s