Handle certificate corruption (#7982)

square · Oct 2, 2023 · cd581af · cd581af
1 parent 708d89b
commit cd581af
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 1 deletion.
diff --git a/okhttp/src/main/kotlin/okhttp3/Cache.kt b/okhttp/src/main/kotlin/okhttp3/Cache.kt
@@ -609,7 +609,8 @@ class Cache internal constructor(
         for (i in 0 until length) {
           val line = source.readUtf8LineStrict()
           val bytes = Buffer()
-          bytes.write(line.decodeBase64()!!)
+          val certificateBytes = line.decodeBase64() ?: throw IOException("Corrupt certificate in cache entry")
+          bytes.write(certificateBytes)
           result.add(certificateFactory.generateCertificate(bytes.inputStream()))
         }
         return result

diff --git a/okhttp/src/test/java/okhttp3/CacheTest.java b/okhttp/src/test/java/okhttp3/CacheTest.java
@@ -296,6 +296,49 @@ private void testResponseCaching(TransferKind transferKind) throws IOException {
     assertThat(response2.handshake().localPrincipal()).isEqualTo(localPrincipal);
   }
 
+  @Test public void secureResponseCachingWithCorruption() throws IOException {
+    server.useHttps(handshakeCertificates.sslSocketFactory());
+    server.enqueue(new MockResponse.Builder()
+            .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))
+            .addHeader("Expires: " + formatDate(1, TimeUnit.HOURS))
+            .body("ABC")
+            .build());
+    server.enqueue(new MockResponse.Builder()
+            .addHeader("Last-Modified: " + formatDate(-5, TimeUnit.MINUTES))
+            .addHeader("Expires: " + formatDate(2, TimeUnit.HOURS))
+            .body("DEF")
+            .build());
+
+    client = client.newBuilder()
+            .sslSocketFactory(
+                    handshakeCertificates.sslSocketFactory(), handshakeCertificates.trustManager())
+            .hostnameVerifier(NULL_HOSTNAME_VERIFIER)
+            .build();
+
+    Request request = new Request.Builder().url(server.url("/")).build();
+    Response response1 = client.newCall(request).execute();
+    assertThat(response1.body().string()).isEqualTo("ABC");
+
+    Path cacheEntry = fileSystem.allPaths().stream()
+            .filter((e) -> e.name().endsWith(".0"))
+            .findFirst()
+            .orElseThrow();
+    corruptCertificate(cacheEntry);
+
+    Response response2 = client.newCall(request).execute(); // Not Cached!
+    assertThat(response2.body().string()).isEqualTo("DEF");
+
+    assertThat(cache.requestCount()).isEqualTo(2);
+    assertThat(cache.networkCount()).isEqualTo(2);
+    assertThat(cache.hitCount()).isEqualTo(0);
+  }
+
+  private void corruptCertificate(Path cacheEntry) throws IOException {
+    String content = Okio.buffer(fileSystem.source(cacheEntry)).readUtf8();
+    content = content.replace("MII", "!!!");
+    Okio.buffer(fileSystem.sink(cacheEntry)).writeUtf8(content).close();
+  }
+
   @Test public void responseCachingAndRedirects() throws Exception {
     server.enqueue(new MockResponse()
         .addHeader("Last-Modified: " + formatDate(-1, TimeUnit.HOURS))