Merge pull request #5637 from Marcono1234/marcono1234/toString-method

Java: Add ToStringMethod

java/ql/src/Security/CWE/CWE-209/StackTraceExposure.ql

@@ -79,8 +79,7 @@ predicate stackTraceExpr(Expr exception, MethodAccess stackTraceString) {
     printStackCall.getAnArgument() = printWriter and
     printStackCall.getQualifier() = exception and
     stackTraceString.getQualifier() = stringWriterVar.getAnAccess() and
-    stackTraceString.getMethod().getName() = "toString" and
+    stackTraceString.getMethod() instanceof ToStringMethod
-    stackTraceString.getMethod().getNumberOfParameters() = 0
   )
 }
 

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

@@ -33,9 +33,8 @@ class InsecureAlgoLiteral extends ShortStringLiteral {
 }
 
 predicate objectToString(MethodAccess ma) {
-  exists(Method m |
+  exists(ToStringMethod m |
     m = ma.getMethod() and
-    m.hasName("toString") and
     m.getDeclaringType() instanceof TypeObject and
     variableTrack(ma.getQualifier()).getType().getErasure() instanceof TypeObject
   )

java/ql/src/Security/CWE/CWE-798/HardcodedCredentialsApiCall.ql

@@ -19,14 +19,14 @@ class HardcodedCredentialApiCallConfiguration extends DataFlow::Configuration {
 
   override predicate isSource(DataFlow::Node n) {
     n.asExpr() instanceof HardcodedExpr and
-    not n.asExpr().getEnclosingCallable().getName() = "toString"
+    not n.asExpr().getEnclosingCallable() instanceof ToStringMethod
   }
 
   override predicate isSink(DataFlow::Node n) { n.asExpr() instanceof CredentialsApiSink }
 
   override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
     node1.asExpr().getType() instanceof TypeString and
-    exists(MethodAccess ma | ma.getMethod().getName().regexpMatch("getBytes|toCharArray") |
+    exists(MethodAccess ma | ma.getMethod().hasName(["getBytes", "toCharArray"]) |
       node2.asExpr() = ma and
       ma.getQualifier() = node1.asExpr()
     )

java/ql/src/Violations of Best Practice/Undesirable Calls/CallsToStringToString.ql

@@ -10,9 +10,8 @@
 
 import java
 
-from MethodAccess ma, Method tostring
+from MethodAccess ma, ToStringMethod tostring
 where
-  tostring.hasName("toString") and
   tostring.getDeclaringType() instanceof TypeString and
   ma.getMethod() = tostring
 select ma, "Redundant call to 'toString' on a String object."

java/ql/src/Violations of Best Practice/Undesirable Calls/DefaultToString.ql

@@ -14,20 +14,13 @@ import java
 import semmle.code.java.StringFormat
 
 predicate explicitToStringCall(Expr e) {
-  exists(MethodAccess ma, Method toString | toString = ma.getMethod() |
+  exists(MethodAccess ma |
-    e = ma.getQualifier() and
+    ma.getMethod() instanceof ToStringMethod and
-    toString.getName() = "toString" and
+    e = ma.getQualifier()
-    toString.getNumberOfParameters() = 0 and
-    not toString.isStatic()
   )
 }
 
-predicate directlyDeclaresToString(Class c) {
+predicate directlyDeclaresToString(Class c) { any(ToStringMethod m).getDeclaringType() = c }
-  exists(Method m | m.getDeclaringType() = c |
-    m.getName() = "toString" and
-    m.getNumberOfParameters() = 0
-  )
-}
 
 predicate inheritsObjectToString(Class t) {
   not directlyDeclaresToString(t.getSourceDeclaration()) and

java/ql/src/experimental/Security/CWE/CWE-297/InsecureLdapEndpoint.ql

@@ -68,7 +68,7 @@ predicate isBooleanTrue(Expr expr) {
   or
   exists(MethodAccess ma |
     expr = ma and
-    ma.getMethod().hasName("toString") and
+    ma.getMethod() instanceof ToStringMethod and
     ma.getQualifier().(FieldAccess).getField().hasName("TRUE") and
     ma.getQualifier()
         .(FieldAccess)

java/ql/src/semmle/code/java/JDK.qll

@@ -353,7 +353,7 @@ class EqualsMethod extends Method {
 class HashCodeMethod extends Method {
   HashCodeMethod() {
     this.hasName("hashCode") and
-    this.getNumberOfParameters() = 0
+    this.hasNoParameters()
   }
 }
 
@@ -365,6 +365,14 @@ class CloneMethod extends Method {
   }
 }
 
+/** A method with the same signature as `java.lang.Object.toString`. */
+class ToStringMethod extends Method {
+  ToStringMethod() {
+    this.hasName("toString") and
+    this.hasNoParameters()
+  }
+}
+
 /**
  * The public static `main` method, with a single formal parameter
  * of type `String[]` and return type `void`.

java/ql/src/semmle/code/java/dispatch/ObjFlow.qll

@@ -194,7 +194,7 @@ private predicate source(RefType t, ObjNode n) {
 private predicate sink(ObjNode n) {
   exists(MethodAccess toString |
     toString.getQualifier() = n.asExpr() and
-    toString.getMethod().hasName("toString")
+    toString.getMethod() instanceof ToStringMethod
   ) and
   n.getTypeBound().getErasure() instanceof TypeObject
 }

java/ql/src/semmle/code/java/security/ControlledString.qll

@@ -8,7 +8,8 @@ import semmle.code.java.Expr
 import semmle.code.java.security.Validation
 
 /**
- * Holds if `method` is a `toString()` method on a boxed type. These never return special characters.
+ * Holds if `method` is a `toString()` method on a boxed type, with or without parameters.
+ * These never return special characters.
  */
 private predicate boxedToString(Method method) {
   method.getDeclaringType() instanceof BoxedType and
@@ -44,11 +45,9 @@ private predicate controlledStringProp(Expr src, Expr dest) {
   exists(AddExpr concatOp | concatOp = dest | src = concatOp.getAnOperand())
   or
   // `toString()` on a safe string is safe.
-  exists(MethodAccess toStringCall, Method toString |
+  exists(MethodAccess toStringCall |
     src = toStringCall.getQualifier() and
-    toString = toStringCall.getMethod() and
+    toStringCall.getMethod() instanceof ToStringMethod and
-    toString.hasName("toString") and
-    toString.getNumberOfParameters() = 0 and
     dest = toStringCall
   )
 }