Merge pull request #351 from nystrom/master

Approved by pavgust

java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.java

@@ -0,0 +1,10 @@
+String s = ...;
+int n;
+
+n = Integer.parseInt(s); // BAD: NumberFormatException is not caught.
+
+try {
+        n = Integer.parseInt(s);
+} catch (NumberFormatException e) {  // GOOD: The exception is caught. 
+        // Handle the exception
+}

java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.qhelp

@@ -0,0 +1,43 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+
+<overview>
+<p>Methods such as <code>Integer.parseInt</code> that parse strings into numbers
+throw 
+<code>NumberFormatException</code> if their arguments cannot be parsed.
+This exception should be caught so that any parse errors can be handled.
+</p>
+
+</overview>
+<recommendation>
+
+<p>It is usually best to handle <code>NumberFormatException</code> in a <code>catch</code> clause
+surrounding the call to the parsing method.</p>
+
+</recommendation>
+<example>
+
+<p>In the following example, the first call to <code>Integer.parseInt</code> does not catch the exception.
+The second call does.
+</p>
+
+<sample src="NumberFormatException.java" />
+
+</example>
+<references>
+
+
+<li>
+  Java Platform, Standard Edition 8, API Specification:
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/Integer.html#valueOf-java.lang.String-">Integer.valueOf</a>,
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/Integer.html#parseInt-java.lang.String-">Integer.parseInt</a>,
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/Long.html#parseLong-java.lang.String-">Long.parseLong</a>,
+  <a href="https://docs.oracle.com/javase/8/docs/api/java/lang/NumberFormatException.html">NumberFormatException</a>.
+</li>
+
+
+</references>
+</qhelp>

java/ql/src/Violations of Best Practice/Exception Handling/NumberFormatException.ql

@@ -0,0 +1,91 @@
+/**
+ * @name Missing catch of NumberFormatException
+ * @description Calling a string to number conversion method without handling
+ *              'NumberFormatException' may cause unexpected runtime exceptions.
+ * @kind problem
+ * @problem.severity recommendation
+ * @precision high
+ * @id java/uncaught-number-format-exception
+ * @tags reliability
+ *       external/cwe/cwe-248
+ */
+
+import java
+
+private class SpecialMethodAccess extends MethodAccess {
+  predicate isValueOfMethod(string klass) {
+    this.getMethod().getName() = "valueOf" and
+    this.getQualifier().getType().(RefType).hasQualifiedName("java.lang", klass) and
+    this.getAnArgument().getType().(RefType).hasQualifiedName("java.lang", "String")
+  }
+
+  predicate isParseMethod(string klass, string name) {
+    this.getMethod().getName() = name and
+    this.getQualifier().getType().(RefType).hasQualifiedName("java.lang", klass)
+  }
+
+  predicate throwsNFE() {
+    this.isParseMethod("Byte", "parseByte") or
+    this.isParseMethod("Short", "parseShort") or
+    this.isParseMethod("Integer", "parseInt") or
+    this.isParseMethod("Long", "parseLong") or
+    this.isParseMethod("Float", "parseFloat") or
+    this.isParseMethod("Double", "parseDouble") or
+    this.isParseMethod("Byte", "decode") or
+    this.isParseMethod("Short", "decode") or
+    this.isParseMethod("Integer", "decode") or
+    this.isParseMethod("Long", "decode") or
+    this.isValueOfMethod("Byte") or
+    this.isValueOfMethod("Short") or
+    this.isValueOfMethod("Integer") or
+    this.isValueOfMethod("Long") or
+    this.isValueOfMethod("Float") or
+    this.isValueOfMethod("Double")
+  }
+}
+
+private class SpecialClassInstanceExpr extends ClassInstanceExpr {
+  predicate isStringConstructor(string klass) {
+    this.getType().(RefType).hasQualifiedName("java.lang", klass) and
+    this.getAnArgument().getType().(RefType).hasQualifiedName("java.lang", "String") and
+    this.getNumArgument() = 1
+  }
+
+  predicate throwsNFE() {
+    this.isStringConstructor("Byte") or
+    this.isStringConstructor("Short") or
+    this.isStringConstructor("Integer") or
+    this.isStringConstructor("Long") or
+    this.isStringConstructor("Float") or
+    this.isStringConstructor("Double")
+  }
+}
+
+class NumberFormatException extends RefType {
+  NumberFormatException() { this.hasQualifiedName("java.lang", "NumberFormatException") }
+}
+
+private predicate catchesNFE(TryStmt t) {
+  exists(CatchClause cc, LocalVariableDeclExpr v |
+    t.getACatchClause() = cc and
+    cc.getVariable() = v and
+    v.getType().(RefType).getASubtype*() instanceof NumberFormatException
+  )
+}
+
+private predicate throwsNFE(Expr e) {
+  e.(SpecialClassInstanceExpr).throwsNFE() or e.(SpecialMethodAccess).throwsNFE()
+}
+
+from Expr e
+where
+  throwsNFE(e) and
+  not exists(TryStmt t |
+    t.getBlock() = e.getEnclosingStmt().getParent*() and
+    catchesNFE(t)
+  ) and
+  not exists(Callable c |
+    e.getEnclosingCallable() = c and
+    c.getAThrownExceptionType().getASubtype*() instanceof NumberFormatException
+  )
+select e, "Potential uncaught 'java.lang.NumberFormatException'."

java/ql/test/query-tests/NumberFormatException/NumberFormatException.expected

@@ -0,0 +1,30 @@
+| Test.java:11:9:11:29 | parseByte(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:12:9:12:26 | decode(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:13:9:13:27 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:14:9:14:31 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:15:9:15:30 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:16:9:16:23 | new Byte(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:19:9:19:31 | parseShort(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:20:9:20:27 | decode(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:21:9:21:28 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:22:9:22:32 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:23:9:23:33 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:24:9:24:24 | new Short(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:27:9:27:31 | parseInt(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:28:9:28:29 | decode(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:29:9:29:30 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:30:9:30:34 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:31:9:31:39 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:32:9:32:26 | new Integer(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:35:9:35:29 | parseLong(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:36:9:36:26 | decode(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:37:9:37:27 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:38:9:38:31 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:39:9:39:36 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:40:9:40:23 | new Long(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:43:9:43:40 | parseFloat(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:44:9:44:37 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:45:9:45:33 | new Float(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:48:9:48:42 | parseDouble(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:49:9:49:38 | valueOf(...) | Potential uncaught 'java.lang.NumberFormatException'. |
+| Test.java:50:9:50:34 | new Double(...) | Potential uncaught 'java.lang.NumberFormatException'. |

java/ql/test/query-tests/NumberFormatException/NumberFormatException.qlref

@@ -0,0 +1 @@
+Violations of Best Practice/Exception Handling/NumberFormatException.ql

java/ql/test/query-tests/NumberFormatException/Test.java

@@ -0,0 +1,136 @@
+import java.io.*;
+
+public class Test {
+    public static void main(String[] args) {
+        test1();
+        test2();
+        test3();
+    }
+
+    static void test1() {
+        Byte.parseByte("123");
+        Byte.decode("123");
+        Byte.valueOf("123");
+        Byte.valueOf("123", 10);
+        Byte.valueOf("7f", 16);
+        new Byte("123");
+        new Byte((byte) 123); // don't flag: wrong constructor
+
+        Short.parseShort("123");
+        Short.decode("123");
+        Short.valueOf("123");
+        Short.valueOf("123", 10);
+        Short.valueOf("7abc", 16);
+        new Short("123");
+        new Short((short) 123); // don't flag: wrong constructor
+
+        Integer.parseInt("123");
+        Integer.decode("123");
+        Integer.valueOf("123");
+        Integer.valueOf("123", 10);
+        Integer.valueOf("1234beef", 16);
+        new Integer("123");
+        new Integer(123); // don't flag: wrong constructor
+
+        Long.parseLong("123");
+        Long.decode("123");
+        Long.valueOf("123");
+        Long.valueOf("123", 10);
+        Long.valueOf("deadbeef", 16);
+        new Long("123");
+        new Long(123l); // don't flag: wrong constructor
+
+        Float.parseFloat("2.7818281828");
+        Float.valueOf("2.7818281828");
+        new Float("2.7818281828");
+        new Float(2.7818281828f); // don't flag: wrong constructor
+
+        Double.parseDouble("2.7818281828");
+        Double.valueOf("2.7818281828");
+        new Double("2.7818281828");
+        new Double(2.7818281828); // don't flag: wrong constructor
+    }
+
+    static void test2() {
+        // Don't flag any of these. The exception is caught.
+        try {
+            Byte.parseByte("123");
+            Byte.decode("123");
+            Byte.valueOf("123");
+            Byte.valueOf("123", 10);
+            Byte.valueOf("7f", 16);
+            new Byte("123");
+
+            Short.parseShort("123");
+            Short.decode("123");
+            Short.valueOf("123");
+            Short.valueOf("123", 10);
+            Short.valueOf("7abc", 16);
+            new Short("123");
+
+            Integer.parseInt("123");
+            Integer.decode("123");
+            Integer.valueOf("123");
+            Integer.valueOf("123", 10);
+            Integer.valueOf("1234beef", 16);
+            new Integer("123");
+
+            Long.parseLong("123");
+            Long.decode("123");
+            Long.valueOf("123");
+            Long.valueOf("123", 10);
+            Long.valueOf("deadbeef", 16);
+            new Long("123");
+
+            Float.parseFloat("2.7818281828");
+            Float.valueOf("2.7818281828");
+            new Float("2.7818281828");
+
+            Double.parseDouble("2.7818281828");
+            Double.valueOf("2.7818281828");
+            new Double("2.7818281828");
+        }
+        catch (NumberFormatException e) {
+            // parse error
+        }
+    }
+
+    static void test3() throws NumberFormatException {
+        // Don't flag any of these: the exception is explcitly declared
+        Byte.parseByte("123");
+        Byte.decode("123");
+        Byte.valueOf("123");
+        Byte.valueOf("123", 10);
+        Byte.valueOf("7f", 16);
+        new Byte("123");
+
+        Short.parseShort("123");
+        Short.decode("123");
+        Short.valueOf("123");
+        Short.valueOf("123", 10);
+        Short.valueOf("7abc", 16);
+        new Short("123");
+
+        Integer.parseInt("123");
+        Integer.decode("123");
+        Integer.valueOf("123");
+        Integer.valueOf("123", 10);
+        Integer.valueOf("1234beef", 16);
+        new Integer("123");
+
+        Long.parseLong("123");
+        Long.decode("123");
+        Long.valueOf("123");
+        Long.valueOf("123", 10);
+        Long.valueOf("deadbeef", 16);
+        new Long("123");
+
+        Float.parseFloat("2.7818281828");
+        Float.valueOf("2.7818281828");
+        new Float("2.7818281828");
+
+        Double.parseDouble("2.7818281828");
+        Double.valueOf("2.7818281828");
+        new Double("2.7818281828");
+    }
+}