From 91015609b22cf3ca54fe6b4f188739122c06f225 Mon Sep 17 00:00:00 2001 From: Daniel Schoepe Date: Tue, 2 Mar 2021 01:39:40 -0800 Subject: [PATCH] [racerd] Add models for javax.crypto.Mac methods (#1395) Summary: The javax.crypto.Mac classes behaves like a container and can lead to race conditions when used in a concurrent context. This adds Mac operations as container writes/reads in RacerD's models. Pull Request resolved: https://github.com/facebook/infer/pull/1395 Test Plan: CI Reviewed By: skcho Differential Revision: D26722737 Pulled By: ngorogiannis fbshipit-source-id: 74f03e9a5 --- infer/src/concurrency/RacerDModels.ml | 8 ++- .../codetoanalyze/java/racerd/Containers.java | 17 +++++++ .../codetoanalyze/java/racerd/issues.exp | 51 ++++++++++--------- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/infer/src/concurrency/RacerDModels.ml b/infer/src/concurrency/RacerDModels.ml index 2b29b0ce0..84be865a8 100644 --- a/infer/src/concurrency/RacerDModels.ml +++ b/infer/src/concurrency/RacerDModels.ml @@ -49,7 +49,9 @@ let is_java_container_write = ; (* https://docs.oracle.com/javase/8/docs/api/java/util/Collection.html *) { default with classname= "java.util.Collection" - ; methods= ["add"; "addAll"; "clear"; "remove"; "removeAll"; "removeIf"] } ] + ; methods= ["add"; "addAll"; "clear"; "remove"; "removeAll"; "removeIf"] } + ; (* https://docs.oracle.com/javase/8/docs/api/javax/crypto/Mac.html *) + {default with classname= "javax.crypto.Mac"; methods= ["update"; "init"; "doFinal"]} ] |> of_records @@ -110,7 +112,9 @@ let is_java_container_read = ; "size" ; "spliterator" ; "stream" - ; "toArray" ] } ] + ; "toArray" ] } + ; (* https://docs.oracle.com/javase/8/docs/api/javax/crypto/Mac.html *) + {default with classname= "javax.crypto.Mac"; methods= ["doFinal"]} ] |> of_records diff --git a/infer/tests/codetoanalyze/java/racerd/Containers.java b/infer/tests/codetoanalyze/java/racerd/Containers.java index 0a813b146..83ad44da4 100644 --- a/infer/tests/codetoanalyze/java/racerd/Containers.java +++ b/infer/tests/codetoanalyze/java/racerd/Containers.java @@ -14,6 +14,8 @@ import android.support.v4.util.SimpleArrayMap; import android.support.v4.util.SparseArrayCompat; import android.util.SparseArray; import com.google.common.collect.Sets; +import java.security.InvalidKeyException; +import java.security.Key; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -27,6 +29,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentSkipListSet; import java.util.concurrent.CopyOnWriteArrayList; import javax.annotation.concurrent.ThreadSafe; +import javax.crypto.Mac; class ContainerWrapper { private final List children = new ArrayList(); @@ -369,4 +372,18 @@ class Containers { void googleSynchronizedSetAddOk(String value) { mGoogleSynchronizedSet.add(value); } + + Mac mac = null; + + void raceOnMacInitBad(Key key) throws InvalidKeyException { + mac.init(key); + } + + void raceOnMacUpdateBad(byte[] bytes) { + mac.update(bytes); + } + + byte[] raceOnMacDoFinalBad() { + return mac.doFinal(); + } } diff --git a/infer/tests/codetoanalyze/java/racerd/issues.exp b/infer/tests/codetoanalyze/java/racerd/issues.exp index 9bb9ddef0..c3da86f38 100644 --- a/infer/tests/codetoanalyze/java/racerd/issues.exp +++ b/infer/tests/codetoanalyze/java/racerd/issues.exp @@ -26,30 +26,33 @@ codetoanalyze/java/racerd/Constructors.java, Constructors.FP_singleton2Ok():Cons codetoanalyze/java/racerd/Constructors.java, Constructors.singleton1Bad():Constructors, 57, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Constructors.(Object),access to `Constructors.staticField`] codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constructors, 63, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,access to `Constructors.sSingleton2`,,access to `Constructors.sSingleton2`] codetoanalyze/java/racerd/Constructors.java, Constructors.singleton2Bad():Constructors, 64, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `Constructors.sSingleton2`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSimpleArrayMapBad(android.support.v4.util.SimpleArrayMap):void, 284, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `map` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayBad(android.util.SparseArray):void, 274, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayCompatBad(android.support.v4.util.SparseArrayCompat):void, 265, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToUnsynchronizedListBad(java.lang.String):void, 326, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mSomeList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.containerWrapperUnownedWriteBad(java.lang.Object):void, 172, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),Write to container `this.mContainerWrapper.children` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getListSizeBad():int, 332, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeOtherList` via call to `size`,,Write to container `this.mSomeOtherList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getMapSizeBad():int, 342, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeMap` via call to `size`,,Write to container `this.mSomeMap` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddAllBad(java.util.Collection):void, 60, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `addAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad1(java.lang.String):void, 52, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad2(int,java.lang.String):void, 56, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listClearBad():void, 64, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `clear`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listReadBad(java.lang.String):boolean, 103, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mListSyncWrites` via call to `contains`,,Write to container `this.mListSyncWrites` via call to `add`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveAllBad(java.util.Collection):void, 76, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `removeAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad1(int):void, 68, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad2(java.lang.String):void, 72, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSetBad(int,java.lang.String):void, 80, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `set`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSubclassWriteBad(java.util.ArrayList,int):void, 84, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `list` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapClearBad():void, 120, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `clear`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutAllBad(java.util.Map):void, 124, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `putAll`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutBad(java.lang.String,java.lang.String):void, 112, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `put`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapRemoveBad(java.lang.String):void, 116, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapSubclassWriteBad(java.util.HashMap,java.lang.String):void, 142, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `m` via call to `remove`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.poolBad():void, 303, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.simplePool` via call to `release`] -codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.readSimpleArrayMap():int, 289, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.si_map` via call to `get`,,Write to container `this.si_map` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSimpleArrayMapBad(android.support.v4.util.SimpleArrayMap):void, 287, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `map` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayBad(android.util.SparseArray):void, 277, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToSparseArrayCompatBad(android.support.v4.util.SparseArrayCompat):void, 268, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `sparseArray` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.addToUnsynchronizedListBad(java.lang.String):void, 329, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mSomeList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.containerWrapperUnownedWriteBad(java.lang.Object):void, 175, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [call to Object ContainerWrapper.write(Object),call to Object ContainerWrapper._write(Object),Write to container `this.mContainerWrapper.children` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getListSizeBad():int, 335, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeOtherList` via call to `size`,,Write to container `this.mSomeOtherList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.getMapSizeBad():int, 345, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mSomeMap` via call to `size`,,Write to container `this.mSomeMap` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddAllBad(java.util.Collection):void, 63, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `addAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad1(java.lang.String):void, 55, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listAddBad2(int,java.lang.String):void, 59, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listClearBad():void, 67, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `clear`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listReadBad(java.lang.String):boolean, 106, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.mListSyncWrites` via call to `contains`,,Write to container `this.mListSyncWrites` via call to `add`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveAllBad(java.util.Collection):void, 79, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `removeAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad1(int):void, 71, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listRemoveBad2(java.lang.String):void, 75, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSetBad(int,java.lang.String):void, 83, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mList` via call to `set`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.listSubclassWriteBad(java.util.ArrayList,int):void, 87, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `list` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapClearBad():void, 123, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `clear`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutAllBad(java.util.Map):void, 127, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `putAll`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapPutBad(java.lang.String,java.lang.String):void, 115, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `put`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapRemoveBad(java.lang.String):void, 119, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mMap` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.mapSubclassWriteBad(java.util.HashMap,java.lang.String):void, 145, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `m` via call to `remove`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.poolBad():void, 306, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.simplePool` via call to `release`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.raceOnMacDoFinalBad():byte[], 387, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mac` via call to `doFinal`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.raceOnMacInitBad(java.security.Key):void, 379, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mac` via call to `init`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.raceOnMacUpdateBad(byte[]):void, 383, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [Write to container `this.mac` via call to `update`] +codetoanalyze/java/racerd/Containers.java, codetoanalyze.java.checkers.Containers.readSimpleArrayMap():int, 292, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [,Read of container `this.si_map` via call to `get`,,Write to container `this.si_map` via call to `put`] codetoanalyze/java/racerd/DeepOwnership.java, DeepOwnership.globalNotOwnedBad():void, 16, THREAD_SAFETY_VIOLATION, no_bucket, WARNING, [access to `DeepOwnership.global.next`] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceBad(codetoanalyze.java.checkers.UnannotatedInterface):void, 49, INTERFACE_NOT_THREAD_SAFE, no_bucket, WARNING, [Call to un-annotated interface method void UnannotatedInterface.foo()] codetoanalyze/java/racerd/Dispatch.java, codetoanalyze.java.checkers.Dispatch.callUnannotatedInterfaceIndirectBad(codetoanalyze.java.checkers.NotThreadSafe,codetoanalyze.java.checkers.UnannotatedInterface):void, 53, INTERFACE_NOT_THREAD_SAFE, no_bucket, WARNING, [call to void NotThreadSafe.notThreadSafeOk(UnannotatedInterface),Call to un-annotated interface method void UnannotatedInterface.foo()]