diff --git a/java/ql/src/experimental/Security/CWE/CWE-1204/BadStaticInitializationVector.java b/java/ql/src/experimental/Security/CWE/CWE-1204/BadStaticInitializationVector.java new file mode 100644 index 000000000000..85e8be6d8ce2 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-1204/BadStaticInitializationVector.java @@ -0,0 +1,4 @@ +byte[] iv = new byte[16]; // all zeroes +GCMParameterSpec params = new GCMParameterSpec(128, iv); +Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); +cipher.init(Cipher.ENCRYPT_MODE, key, params); \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-1204/GoodRandomInitializationVector.java b/java/ql/src/experimental/Security/CWE/CWE-1204/GoodRandomInitializationVector.java new file mode 100644 index 000000000000..faceb119d643 --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-1204/GoodRandomInitializationVector.java @@ -0,0 +1,6 @@ +byte[] iv = new byte[16]; +SecureRandom random = SecureRandom.getInstanceStrong(); +random.nextBytes(iv); +GCMParameterSpec params = new GCMParameterSpec(128, iv); +Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); +cipher.init(Cipher.ENCRYPT_MODE, key, params); \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.qhelp b/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.qhelp new file mode 100644 index 000000000000..d631dff22aff --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.qhelp @@ -0,0 +1,46 @@ + + + + +

+A cipher needs an initialization vector (IV) when it is used in certain modes +such as CBC or GCM. Under the same secret key, IVs should be unique and ideally unpredictable. +Given a secret key, if the same IV is used for encryption, the same plaintexts result in the same ciphertexts. +This lets an attacker learn if the same data pieces are transferred or stored, +or this can help the attacker run a dictionary attack. +

+
+ + +

+Use a random IV generated by SecureRandom. +

+
+ + +

+The following example initializes a cipher with a static IV which is unsafe: +

+ + +

+The next example initializes a cipher with a random IV: +

+ +
+ + +
  • + Wikipedia: + Initialization vector. +
  • +
  • + National Institute of Standards and Technology: + Recommendation for Block Cipher Modes of Operation. +
  • +
  • + National Institute of Standards and Technology: + FIPS 140-2: Security Requirements for Cryptographic Modules. +
  • +
    +
    \ No newline at end of file diff --git a/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.ql b/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.ql new file mode 100644 index 000000000000..9980f68ed80f --- /dev/null +++ b/java/ql/src/experimental/Security/CWE/CWE-1204/StaticInitializationVector.ql @@ -0,0 +1,26 @@ +/** + * @name Using a static initialization vector for encryption + * @description A cipher needs an initialization vector (IV) in some cases, + * for example, when CBC or GCM modes are used. IVs are used to randomize the encryption, + * therefore they should be unique and ideally unpredictable. + * Otherwise, the same plaintexts result in same ciphertexts under a given secret key. + * If a static IV is used for encryption, this lets an attacker learn + * if the same data pieces are transferred or stored, + * or this can help the attacker run a dictionary attack. + * @kind path-problem + * @problem.severity warning + * @precision high + * @id java/static-initialization-vector + * @tags security + * external/cwe/cwe-329 + * external/cwe/cwe-1204 + */ + +import java +import experimental.semmle.code.java.security.StaticInitializationVectorQuery +import DataFlow::PathGraph + +from DataFlow::PathNode source, DataFlow::PathNode sink, StaticInitializationVectorConfig conf +where conf.hasFlowPath(source, sink) +select sink.getNode(), source, sink, "A $@ should not be used for encryption.", source.getNode(), + "static initialization vector" diff --git a/java/ql/src/experimental/semmle/code/java/security/StaticInitializationVectorQuery.qll b/java/ql/src/experimental/semmle/code/java/security/StaticInitializationVectorQuery.qll new file mode 100644 index 000000000000..5d88d620c214 --- /dev/null +++ b/java/ql/src/experimental/semmle/code/java/security/StaticInitializationVectorQuery.qll @@ -0,0 +1,165 @@ +import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.TaintTracking2 + +/** + * Holds if `array` is initialized only with constants. + */ +private predicate initializedWithConstants(ArrayCreationExpr array) { + // creating an array without an initializer, for example `new byte[8]` + not exists(array.getInit()) + or + // creating a multidimensional array with an initializer like `{ new byte[8], new byte[16] }` + // This works around https://github.com/github/codeql/issues/6552 -- change me once there is + // a better way to distinguish nested initializers that create zero-filled arrays + // (e.g. `new byte[1]`) from those with an initializer list (`new byte[] { 1 }` or just `{ 1 }`) + array.getInit().getAnInit().getAChildExpr() instanceof IntegerLiteral + or + // creating an array wit an initializer like `new byte[] { 1, 2 }` + forex(Expr element | element = array.getInit().getAnInit() | + element instanceof CompileTimeConstantExpr + ) +} + +/** + * An expression that creates a byte array that is initialized with constants. + */ +private class StaticByteArrayCreation extends ArrayCreationExpr { + StaticByteArrayCreation() { + this.getType().(Array).getElementType().(PrimitiveType).getName() = "byte" and + initializedWithConstants(this) + } +} + +/** An expression that updates `array`. */ +private class ArrayUpdate extends Expr { + Expr array; + + ArrayUpdate() { + exists(Assignment assign | + assign = this and + assign.getDest().(ArrayAccess).getArray() = array and + not assign.getSource() instanceof CompileTimeConstantExpr + ) + or + exists(StaticMethodAccess ma | + ma.getMethod().hasQualifiedName("java.lang", "System", "arraycopy") and + ma = this and + ma.getArgument(2) = array + ) + or + exists(MethodAccess ma, Method m | + m = ma.getMethod() and + ma = this and + ma.getArgument(0) = array + | + m.hasQualifiedName("java.io", "InputStream", "read") or + m.hasQualifiedName("java.nio", "ByteBuffer", "get") or + m.hasQualifiedName("java.security", "SecureRandom", "nextBytes") or + m.hasQualifiedName("java.util", "Random", "nextBytes") + ) + } + + /** Returns the updated array. */ + Expr getArray() { result = array } +} + +/** + * A config that tracks dataflow from creating an array to an operation that updates it. + */ +private class ArrayUpdateConfig extends TaintTracking2::Configuration { + ArrayUpdateConfig() { this = "ArrayUpdateConfig" } + + override predicate isSource(DataFlow::Node source) { + source.asExpr() instanceof StaticByteArrayCreation + } + + override predicate isSink(DataFlow::Node sink) { + exists(ArrayUpdate update | update.getArray() = sink.asExpr()) + } +} + +/** + * A source that defines an array that doesn't get updated. + */ +private class StaticInitializationVectorSource extends DataFlow::Node { + StaticInitializationVectorSource() { + exists(StaticByteArrayCreation array | array = this.asExpr() | + not exists(ArrayUpdateConfig config | config.hasFlow(DataFlow2::exprNode(array), _)) + ) + } +} + +/** + * A config that tracks initialization of a cipher for encryption. + */ +private class EncryptionModeConfig extends TaintTracking2::Configuration { + EncryptionModeConfig() { this = "EncryptionModeConfig" } + + override predicate isSource(DataFlow::Node source) { + source + .asExpr() + .(FieldRead) + .getField() + .hasQualifiedName("javax.crypto", "Cipher", "ENCRYPT_MODE") + } + + override predicate isSink(DataFlow::Node sink) { + exists(MethodAccess ma, Method m | m = ma.getMethod() | + m.hasQualifiedName("javax.crypto", "Cipher", "init") and + ma.getArgument(0) = sink.asExpr() + ) + } +} + +/** + * A sink that initializes a cipher for encryption with unsafe parameters. + */ +private class EncryptionInitializationSink extends DataFlow::Node { + EncryptionInitializationSink() { + exists(MethodAccess ma, Method m, EncryptionModeConfig config | m = ma.getMethod() | + m.hasQualifiedName("javax.crypto", "Cipher", "init") and + m.getParameterType(2) + .(RefType) + .hasQualifiedName("java.security.spec", "AlgorithmParameterSpec") and + ma.getArgument(2) = this.asExpr() and + config.hasFlowToExpr(ma.getArgument(0)) + ) + } +} + +/** + * Holds if `fromNode` to `toNode` is a dataflow step + * that creates cipher's parameters with initialization vector. + */ +private predicate createInitializationVectorSpecStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + exists(ConstructorCall cc, RefType type | + cc = toNode.asExpr() and type = cc.getConstructedType() + | + type.hasQualifiedName("javax.crypto.spec", "IvParameterSpec") and + cc.getArgument(0) = fromNode.asExpr() + or + type.hasQualifiedName("javax.crypto.spec", ["GCMParameterSpec", "RC2ParameterSpec"]) and + cc.getArgument(1) = fromNode.asExpr() + or + type.hasQualifiedName("javax.crypto.spec", "RC5ParameterSpec") and + cc.getArgument(3) = fromNode.asExpr() + ) +} + +/** + * A config that tracks dataflow to initializing a cipher with a static initialization vector. + */ +class StaticInitializationVectorConfig extends TaintTracking::Configuration { + StaticInitializationVectorConfig() { this = "StaticInitializationVectorConfig" } + + override predicate isSource(DataFlow::Node source) { + source instanceof StaticInitializationVectorSource + } + + override predicate isSink(DataFlow::Node sink) { sink instanceof EncryptionInitializationSink } + + override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) { + createInitializationVectorSpecStep(fromNode, toNode) + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.java b/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.java new file mode 100644 index 000000000000..f964d17239bb --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVector.java @@ -0,0 +1,167 @@ +import javax.crypto.Cipher; +import javax.crypto.spec.GCMParameterSpec; +import javax.crypto.spec.IvParameterSpec; +import javax.crypto.spec.SecretKeySpec; + +import java.security.SecureRandom; +import java.util.Arrays; + +public class StaticInitializationVector { + + // BAD: AES-GCM with static IV from a byte array + public byte[] encryptWithStaticIvByteArrayWithInitializer(byte[] key, byte[] plaintext) throws Exception { + byte[] iv = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }; + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // BAD: AES-GCM with static IV from zero-initialized byte array + public byte[] encryptWithZeroStaticIvByteArray(byte[] key, byte[] plaintext) throws Exception { + byte[] iv = new byte[16]; + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // BAD: AES-CBC with static IV from zero-initialized byte array + public byte[] encryptWithStaticIvByteArray(byte[] key, byte[] plaintext) throws Exception { + byte[] iv = new byte[16]; + for (byte i = 0; i < iv.length; i++) { + iv[i] = 1; + } + + IvParameterSpec ivSpec = new IvParameterSpec(iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/CBC/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // BAD: AES-GCM with static IV from a multidimensional byte array + public byte[] encryptWithOneOfStaticIvs01(byte[] key, byte[] plaintext) throws Exception { + byte[][] staticIvs = new byte[][] { + { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }, + { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 42 } + }; + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, staticIvs[1]); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // BAD: AES-GCM with static IV from a multidimensional byte array + public byte[] encryptWithOneOfStaticIvs02(byte[] key, byte[] plaintext) throws Exception { + byte[][] staticIvs = new byte[][] { + new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 5 }, + new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 0, 1, 2, 3, 4, 42 } + }; + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, staticIvs[1]); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // BAD: AES-GCM with static IV from a multidimensional byte array + public byte[] encryptWithOneOfStaticZeroIvs(byte[] key, byte[] plaintext) throws Exception { + byte[][] ivs = new byte[][] { + new byte[8], + new byte[16] + }; + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, ivs[1]); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); // $staticInitializationVector + cipher.update(plaintext); + return cipher.doFinal(); + } + + // GOOD: AES-GCM with a random IV + public byte[] encryptWithRandomIv(byte[] key, byte[] plaintext) throws Exception { + byte[] iv = new byte[16]; + + SecureRandom random = SecureRandom.getInstanceStrong(); + random.nextBytes(iv); + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); + cipher.update(plaintext); + return cipher.doFinal(); + } + + // GOOD: AES-GCM with a random IV + public byte[] encryptWithRandomIvByteByByte(byte[] key, byte[] plaintext) throws Exception { + SecureRandom random = SecureRandom.getInstanceStrong(); + byte[] iv = new byte[16]; + for (int i = 0; i < iv.length; i++) { + iv[i] = (byte) random.nextInt(); + } + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); + cipher.update(plaintext); + return cipher.doFinal(); + } + + // GOOD: AES-GCM with a random IV + public byte[] encryptWithRandomIvWithSystemArrayCopy(byte[] key, byte[] plaintext) throws Exception { + byte[] randomBytes = new byte[16]; + SecureRandom.getInstanceStrong().nextBytes(randomBytes); + + byte[] iv = new byte[16]; + System.arraycopy(randomBytes, 0, iv, 0, 16); + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); + cipher.update(plaintext); + return cipher.doFinal(); + } + + // GOOD: AES-GCM with a random IV + public byte[] encryptWithRandomIvWithArraysCopy(byte[] key, byte[] plaintext) throws Exception { + byte[] randomBytes = new byte[16]; + SecureRandom.getInstanceStrong().nextBytes(randomBytes); + + byte[] iv = new byte[16]; + iv = Arrays.copyOf(randomBytes, 16); + + GCMParameterSpec ivSpec = new GCMParameterSpec(128, iv); + SecretKeySpec keySpec = new SecretKeySpec(key, "AES"); + + Cipher cipher = Cipher.getInstance("AES/GCM/PKCS5PADDING"); + cipher.init(Cipher.ENCRYPT_MODE, keySpec, ivSpec); + cipher.update(plaintext); + return cipher.doFinal(); + } +} diff --git a/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.expected b/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql b/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql new file mode 100644 index 000000000000..29afc31ab045 --- /dev/null +++ b/java/ql/test/experimental/query-tests/security/CWE-1204/StaticInitializationVectorTest.ql @@ -0,0 +1,20 @@ +import java +import experimental.semmle.code.java.security.StaticInitializationVectorQuery +import TestUtilities.InlineExpectationsTest + +class StaticInitializationVectorTest extends InlineExpectationsTest { + StaticInitializationVectorTest() { this = "StaticInitializationVectorTest" } + + override string getARelevantTag() { result = "staticInitializationVector" } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + tag = "staticInitializationVector" and + exists(DataFlow::Node src, DataFlow::Node sink, StaticInitializationVectorConfig conf | + conf.hasFlow(src, sink) + | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + } +}