CVE-2020-8929 (Other, published 2020-10-19T13:15Z)
Description: value: A mis-handling of invalid unicode characters in the Java implementation of Tink versions prior to 1.5 allows an attacker to change the ID part of a ciphertext, which result in the creation of a second ciphertext that can decrypt to the same plaintext. This can be a problem with encrypting deterministic AEAD with a single key, and rely on a unique ciphertext-per-plaintext.
Severity: MEDIUM (CVSS v3 base score: 5.3)
Weakness: NVD-CWE-Other \u2014 Other
NVD is only using a subset of CWE for mapping instead of the entire CWE, and the weakness type is not covered by that subset.
Repository: https://github.com/google/tink
Files changed:
java_src/src/main/java/com/google/crypto/tink/BUILD.bazel
java_src/src/main/java/com/google/crypto/tink/PrimitiveSet.java
java_src/src/test/java/com/google/crypto/tink/BUILD.bazel
java_src/src/test/java/com/google/crypto/tink/PrimitiveSetTest.java
Commit message: Fixing ciphertext malleability issue in Java caused by storing the ciphertext prefix in a hashmap keyed by UTF8 encoded strings, instead of byte arrays, leading to the ability to retrieve keys with IDs that happen to be invalid Unicode strings with a changed ID.

PiperOrigin-RevId: 336763863


Vulnerable code:
import java.nio.charset.Charset;
  private static final Charset UTF_8 = Charset.forName("UTF-8");
    List<Entry<P>> found = primitives.get(new String(identifier, UTF_8));
  private ConcurrentMap<java.lang.String, List<Entry<P>>> primitives =
      new ConcurrentHashMap<java.lang.String, List<Entry<P>>>();
    // Cannot use [] as keys in hash map, convert to string.
    String identifier = new String(entry.getIdentifier(), UTF_8);
import java.nio.charset.Charset;
  private static final Charset UTF_8 = Charset.forName("UTF-8");
    assertEquals(3, pset.getAll().size());
    assertEquals(1, entries.size());
        DummyMac1.class.getSimpleName(),
        new String(entry.getPrimitive().computeMac(null), "UTF-8"));
    assertEquals(1, entries.size());
        new String(entry.getPrimitive().computeMac(null), "UTF-8"));
    assertEquals(0, entry.getIdentifier().length);
    assertEquals(1, entries.size());
        DummyMac1.class.getSimpleName(),
        new String(entry.getPrimitive().computeMac(null), "UTF-8"));
        new String(entry.getPrimitive().computeMac(null), "UTF-8"));

Fixed code:
        "//src/main/java/com/google/crypto/tink/subtle:hex",
        "//src/main/java/com/google/crypto/tink/subtle:hex",
import com.google.crypto.tink.subtle.Hex;
    List<Entry<P>> found = primitives.get(new Prefix(identifier));
  private final ConcurrentMap<Prefix, List<Entry<P>>> primitives =
      new ConcurrentHashMap<Prefix, List<Entry<P>>>();
    // Cannot use [] as keys in hash map, convert to Prefix wrapper class.
    Prefix identifier = new Prefix(entry.getIdentifier());

  private static class Prefix implements Comparable<Prefix> {
    private final byte[] prefix;

    private Prefix(byte[] prefix) {
      this.prefix = Arrays.copyOf(prefix, prefix.length);
    }

    @Override
    public int hashCode() {
      return Arrays.hashCode(prefix);
    }

    @Override
    public boolean equals(Object o) {
      if (!(o instanceof Prefix)) {
        return false;
      }
      Prefix other = (Prefix) o;
      return Arrays.equals(prefix, other.prefix);
    }

    @Override
    public int compareTo(Prefix o) {
      if (prefix.length != o.prefix.length) {
        return prefix.length - o.prefix.length;
      }
      for (int i = 0; i < prefix.length; i++) {
        if (prefix[i] != o.prefix[i]) {
          return prefix[i] - o.prefix[i];
        }
      }
      return 0;
    }

    @Override
    public String toString() {
      return Hex.encode(prefix);
    }
  }

        "//src/main/java/com/google/crypto/tink/subtle:hex",
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.crypto.tink.subtle.Hex;
    assertThat(pset.getAll()).hasSize(3);
    assertThat(entries).hasSize(1);
        DummyMac1.class.getSimpleName(), new String(entry.getPrimitive().computeMac(null), UTF_8));
    assertThat(entries).hasSize(1);
        new String(entry.getPrimitive().computeMac(null), UTF_8));
    assertThat(entry.getIdentifier()).isEmpty();
    assertThat(entries).hasSize(1);
        DummyMac1.class.getSimpleName(), new String(entry.getPrimitive().computeMac(null), UTF_8));
        new String(entry.getPrimitive().computeMac(null), UTF_8));

  @Test
  public void testPrefix_isUnique() throws Exception {
    PrimitiveSet<Mac> pset = PrimitiveSet.newPrimitiveSet(Mac.class);
    Key key1 =
        Key.newBuilder()
            .setKeyId(0xffffffff)
            .setStatus(KeyStatusType.ENABLED)
            .setOutputPrefixType(OutputPrefixType.TINK)
            .build();
    pset.addPrimitive(new DummyMac1(), key1);
    Key key2 =
        Key.newBuilder()
            .setKeyId(0xffffffdf)
            .setStatus(KeyStatusType.ENABLED)
            .setOutputPrefixType(OutputPrefixType.RAW)
            .build();
    pset.setPrimary(pset.addPrimitive(new DummyMac2(), key2));
    Key key3 =
        Key.newBuilder()
            .setKeyId(0xffffffef)
            .setStatus(KeyStatusType.ENABLED)
            .setOutputPrefixType(OutputPrefixType.LEGACY)
            .build();
    pset.addPrimitive(new DummyMac1(), key3);

    assertThat(pset.getAll()).hasSize(3);

    assertThat(pset.getPrimitive(Hex.decode("01ffffffff"))).hasSize(1);
    assertThat(pset.getPrimitive(Hex.decode("01ffffffef"))).isEmpty();
    assertThat(pset.getPrimitive(Hex.decode("00ffffffff"))).isEmpty();
    assertThat(pset.getPrimitive(Hex.decode("00ffffffef"))).hasSize(1);
  }