Throwable.getMessage is unsafe by default

baseline-error-prone/src/main/java/com/palantir/baseline/errorprone/safety/SafetyPropagationTransfer.java

@@ -137,6 +137,14 @@ public final class SafetyPropagationTransfer implements ForwardTransferFunction<
     private static final Matcher<ExpressionTree> TO_STRING =
             MethodMatchers.instanceMethod().anyClass().named("toString").withNoParameters();
 
+    private static final Matcher<ExpressionTree> THROWABLE_GET_MESSAGE = MethodMatchers.instanceMethod()
+            .onDescendantOf(Throwable.class.getName())
+            .namedAnyOf("getMessage", "getLocalizedMessage")
+            .withNoParameters();
+
+    private static final Matcher<ExpressionTree> STRING_FORMAT =
+            MethodMatchers.staticMethod().onClass(String.class.getName()).named("format");
+
     private VisitorState state;
     private final Set<VarSymbol> traversed = new HashSet<>();
 
@@ -770,24 +778,42 @@ public TransferResult<Safety, AccessPathStore<Safety>> visitCase(
     @Override
     public TransferResult<Safety, AccessPathStore<Safety>> visitMethodInvocation(
             MethodInvocationNode node, TransferInput<Safety, AccessPathStore<Safety>> input) {
-        Safety result = getMethodSymbolSafety(node, input);
+        Safety methodSymbolSafety = getMethodSymbolSafety(node, input);
+        Safety knownMethodSafety = getKnownMethodSafety(node, input);
+        Safety result = Safety.mergeAssumingUnknownIsSame(methodSymbolSafety, knownMethodSafety);
         return noStoreChanges(result, input);
     }
 
+    private Safety getKnownMethodSafety(
+            MethodInvocationNode node, TransferInput<Safety, AccessPathStore<Safety>> input) {
+        if (THROWABLE_GET_MESSAGE.matches(node.getTree(), state)) {
+            // Auth failures are sometimes annotated '@DoNotLog', which getMessage should inherit.
+            return Safety.mergeAssumingUnknownIsSame(
+                    Safety.UNSAFE, input.getValueOfSubNode(node.getTarget().getReceiver()));
+        } else if (STRING_FORMAT.matches(node.getTree(), state)) {
+            Safety safety = Safety.SAFE;
+            for (Node argument : node.getArguments()) {
+                safety = safety.leastUpperBound(input.getValueOfSubNode(argument));
+            }
+            return safety;
+        }
+        return Safety.UNKNOWN;
+    }
+
     private Safety getMethodSymbolSafety(
             MethodInvocationNode node, TransferInput<Safety, AccessPathStore<Safety>> input) {
+        Safety resultTypeSafety = SafetyAnnotations.getResultTypeSafety(node.getTree(), state);
         MethodSymbol methodSymbol = ASTHelpers.getSymbol(node.getTree());
         if (methodSymbol != null) {
             Safety methodSafety = Safety.mergeAssumingUnknownIsSame(
-                    SafetyAnnotations.getSafety(methodSymbol, state),
-                    SafetyAnnotations.getResultTypeSafety(node.getTree(), state));
+                    SafetyAnnotations.getSafety(methodSymbol, state), resultTypeSafety);
             // non-annotated toString inherits type-level safety.
             if (methodSafety == Safety.UNKNOWN && TO_STRING.matches(node.getTree(), state)) {
                 return input.getValueOfSubNode(node.getTarget().getReceiver());
             }
             return methodSafety;
         }
-        return SafetyAnnotations.getResultTypeSafety(node.getTree(), state);
+        return resultTypeSafety;
     }
 
     @Override
@@ -806,7 +832,11 @@ public TransferResult<Safety, AccessPathStore<Safety>> visitMemberReference(
     @Override
     public TransferResult<Safety, AccessPathStore<Safety>> visitArrayCreation(
             ArrayCreationNode node, TransferInput<Safety, AccessPathStore<Safety>> input) {
-        return unknown(input);
+        Safety safety = Safety.SAFE;
+        for (Node item : node.getInitializers()) {
+            safety = safety.leastUpperBound(input.getValueOfSubNode(item));
+        }
+        return noStoreChanges(safety, input);
     }
 
     @Override

baseline-error-prone/src/test/java/com/palantir/baseline/errorprone/IllegalSafeLoggingArgumentTest.java

@@ -635,6 +635,90 @@ public void testInvalidValue_AnnotatedSupertype() {
                 .doTest();
     }
 
+    @Test
+    public void testThrowableMessageIsUnsafe() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  void f(IllegalStateException e) {",
+                        "    String message = e.getMessage();",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(message);",
+                        "    String localized = e.getLocalizedMessage();",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(localized);",
+                        "  }",
+                        "  private static void fun(@Safe Object obj) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testThrowableMessageInheritsDoNotLogFromThrowable() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  void f(@DoNotLog IllegalStateException e) {",
+                        "    String message = e.getMessage();",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(message);",
+                        "  }",
+                        "  private static void fun(@Safe Object obj) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testStringFormat() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  void f(@Safe String safeParam, @Unsafe String unsafeParam, @DoNotLog String dnlParam) {",
+                        "    fun(String.format(\"%s\", safeParam));",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(String.format(\"%s\", unsafeParam));",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(String.format(\"%s\", dnlParam));",
+                        "  }",
+                        "  private static void fun(@Safe Object obj) {}",
+                        "}")
+                .doTest();
+    }
+
+    @Test
+    public void testArraySafety() {
+        helper().addSourceLines(
+                        "Test.java",
+                        "import com.palantir.logsafe.*;",
+                        "class Test {",
+                        "  void f(@Safe String safeParam, @Unsafe String unsafeParam, @DoNotLog String dnlParam) {",
+                        "    Object[] one = new Object[3];",
+                        "    fun(one);",
+                        "    one[0] = unsafeParam;",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'UNSAFE' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(one);",
+                        "    one[1] = dnlParam;",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(one);",
+                        "    // BUG: Diagnostic contains: Dangerous argument value: arg is 'DO_NOT_LOG' "
+                                + "but the parameter requires 'SAFE'.",
+                        "    fun(new Object[] {safeParam, unsafeParam, dnlParam});",
+                        "  }",
+                        "  private static void fun(@Safe Object obj) {}",
+                        "}")
+                .doTest();
+    }
+
     @Test
     public void testSafeArgOfUnsafe_recommendsUnsafeArgOf() {
         refactoringHelper()

changelog/@unreleased/pr-2151.v2.yml

@@ -0,0 +1,5 @@
+type: improvement
+improvement:
+  description: Throwable.getMessage is unsafe by default
+  links:
+  - https://github.com/palantir/gradle-baseline/pull/2151