Handle invalid base64 in SRI

Previously, invalid base64 would crash the Bazel server. Closes bazelbuild#23410. PiperOrigin-RevId: 667624565 Change-Id: I7b6b834fd291e9a3dae59df88b29bb378e7663a3
iancha1992 · Aug 26, 2024 · 1b0ed28 · 1b0ed28
1 parent 05013c1
commit 1b0ed28
Show file tree

Hide file tree

Showing 2 changed files with 39 additions and 20 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/downloader/Checksum.java
@@ -31,6 +31,10 @@ private InvalidChecksumException(KeyType keyType, String hash) {
     private InvalidChecksumException(String msg) {
       super(msg);
     }
+
+    private InvalidChecksumException(String msg, Throwable cause) {
+      super(msg, cause);
+    }
   }
 
   /** Exception thrown to indicate that a checksum is missing. */
@@ -64,41 +68,42 @@ private static Checksum fromString(KeyType keyType, String hash, boolean useSubr
         keyType, HashCode.fromString(Ascii.toLowerCase(hash)), useSubresourceIntegrity);
   }
 
+  private static byte[] base64Decode(String data) throws InvalidChecksumException {
+    try {
+      return Base64.getDecoder().decode(data);
+    } catch (IllegalArgumentException e) {
+      throw new InvalidChecksumException("Invalid base64 '" + data + "'", e);
+    }
+  }
+
   /** Constructs a new Checksum from a hash in Subresource Integrity format. */
   public static Checksum fromSubresourceIntegrity(String integrity)
       throws InvalidChecksumException {
-    Base64.Decoder decoder = Base64.getDecoder();
-    KeyType keyType = null;
-    byte[] hash = null;
-    int expectedLength = 0;
+    KeyType keyType;
+    byte[] hash;
+    int expectedLength;
 
     if (integrity.startsWith("sha1-")) {
       keyType = KeyType.SHA1;
       expectedLength = 20;
-      hash = decoder.decode(integrity.substring(5));
-    }
-    if (integrity.startsWith("sha256-")) {
+      hash = base64Decode(integrity.substring(5));
+    } else if (integrity.startsWith("sha256-")) {
       keyType = KeyType.SHA256;
       expectedLength = 32;
-      hash = decoder.decode(integrity.substring(7));
-    }
-    if (integrity.startsWith("sha384-")) {
+      hash = base64Decode(integrity.substring(7));
+    } else if (integrity.startsWith("sha384-")) {
       keyType = KeyType.SHA384;
       expectedLength = 48;
-      hash = decoder.decode(integrity.substring(7));
-    }
-    if (integrity.startsWith("sha512-")) {
+      hash = base64Decode(integrity.substring(7));
+    } else if (integrity.startsWith("sha512-")) {
       keyType = KeyType.SHA512;
       expectedLength = 64;
-      hash = decoder.decode(integrity.substring(7));
-    }
-    if (integrity.startsWith("blake3-")) {
+      hash = base64Decode(integrity.substring(7));
+    } else if (integrity.startsWith("blake3-")) {
       keyType = KeyType.BLAKE3;
       expectedLength = 32;
-      hash = decoder.decode(integrity.substring(7));
-    }
-
-    if (keyType == null) {
+      hash = base64Decode(integrity.substring(7));
+    } else {
       throw new InvalidChecksumException(
           "Unsupported checksum algorithm: '"
               + integrity

diff --git a/src/test/shell/bazel/external_integration_test.sh b/src/test/shell/bazel/external_integration_test.sh
@@ -1326,6 +1326,20 @@ EOF
   shutdown_server
 }
 
+function test_integrity_ill_formed_base64() {
+  cat > $(setup_module_dot_bazel) <<EOF
+http_archive = use_repo_rule("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
+http_archive(
+    name = "repo",
+    integrity = "sha256-Yab3Yqr2BlLL8zKHm43MLP2BviEpoGHalX0Dnq538L=",
+    url = "file:///dev/null",
+)
+EOF
+  bazel build @repo//... &> $TEST_log 2>&1 && fail "Expected to fail"
+  expect_log "Invalid base64 'Yab3Yqr2BlLL8zKHm43MLP2BviEpoGHalX0Dnq538L='"
+  shutdown_server
+}
+
 function test_same_name() {
   mkdir ext
   echo foo> ext/foo