KTOR-7727 Backport fix for CVE-2024-49580 to Ktor 2 (#4465)

Co-authored-by: Leonid Stashevsky <[email protected]>

ktor-client/ktor-client-core/build.gradle.kts

@@ -1,3 +1,7 @@
+/*
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
+
 description = "Ktor http client"
 
 kotlin.sourceSets {
@@ -29,4 +33,10 @@ kotlin.sourceSets {
             api(project(":ktor-client:ktor-client-mock"))
         }
     }
+
+    jvmTest {
+        dependencies {
+            api(project(":ktor-server:ktor-server-test-host"))
+        }
+    }
 }

ktor-client/ktor-client-core/common/src/io/ktor/client/plugins/cache/HttpCache.kt

@@ -1,6 +1,6 @@
 /*
-* Copyright 2014-2021 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
-*/
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
 
 @file:Suppress("DEPRECATION")
 
@@ -48,7 +48,7 @@ public class HttpCache private constructor(
     private val publicStorageNew: CacheStorage,
     private val privateStorageNew: CacheStorage,
     private val useOldStorage: Boolean,
-    internal val isSharedClient: Boolean
+    internal val isSharedClient: Boolean,
 ) {
     /**
      * A configuration for the [HttpCache] plugin.
@@ -141,6 +141,10 @@ public class HttpCache private constructor(
                 if (content !is OutgoingContent.NoContent) return@intercept
                 if (context.method != HttpMethod.Get || !context.url.protocol.canStore()) return@intercept
 
+                if (plugin.isSharedClient && context.headers.contains(HttpHeaders.Authorization)) {
+                    return@intercept
+                }
+
                 if (plugin.useOldStorage) {
                     interceptSendLegacy(plugin, content, scope)
                     return@intercept
@@ -349,6 +353,7 @@ internal fun mergedHeadersLookup(
         HttpHeaders.UserAgent -> {
             content.headers[HttpHeaders.UserAgent] ?: headerExtractor(HttpHeaders.UserAgent) ?: KTOR_DEFAULT_USER_AGENT
         }
+
         else -> {
             val value = content.headers.getAll(header) ?: allHeadersExtractor(header) ?: emptyList()
             value.joinToString(";")

ktor-client/ktor-client-core/jvm/test/HttpCacheTest.kt

@@ -0,0 +1,123 @@
+/*
+ * Copyright 2014-2024 JetBrains s.r.o and contributors. Use of this source code is governed by the Apache 2.0 license.
+ */
+
+import io.ktor.client.plugins.cache.*
+import io.ktor.client.request.*
+import io.ktor.client.statement.*
+import io.ktor.http.*
+import io.ktor.server.response.*
+import io.ktor.server.routing.*
+import io.ktor.server.testing.*
+import kotlinx.coroutines.*
+import kotlin.test.*
+import kotlin.time.Duration.Companion.milliseconds
+
+class HttpCacheTest {
+
+    @Test
+    fun `should not mix ETags when Authorization header is present`() = testApplication {
+        application {
+            routing {
+                get("/me") {
+                    val user = context.request.headers["Authorization"]!!
+                    if (user == "user-a") {
+                        // Simulate slower network for one of the requests
+                        delay(100.milliseconds)
+                    }
+                    val etag = "etag-of-$user"
+                    if (context.request.headers["If-None-Match"] == etag) {
+                        context.respond(HttpStatusCode.NotModified)
+                        return@get
+                    }
+                    context.response.header("Cache-Control", "no-cache")
+                    context.response.header("ETag", etag)
+                    context.respondText(user)
+                }
+            }
+        }
+
+        val client = createClient {
+            install(HttpCache) {
+                isShared = true
+            }
+        }
+
+        assertEquals(
+            client.get("/me") {
+                headers["Authorization"] = "user-a"
+            }.bodyAsText(),
+            "user-a"
+        )
+        withContext(Dispatchers.Default) {
+            listOf(
+                launch {
+                    val response = client.get("/me") {
+                        headers["Authorization"] = "user-a"
+                    }.bodyAsText()
+
+                    assertEquals("user-a", response)
+                },
+                launch {
+                    val response = client.get("/me") {
+                        headers["Authorization"] = "user-b"
+                    }.bodyAsText()
+
+                    assertEquals("user-b", response)
+                }
+            ).joinAll()
+        }
+    }
+
+    @Test
+    fun `should mix ETags when Authorization header is present and client is not shared`() = testApplication {
+        application {
+            routing {
+                get("/me") {
+                    val user = context.request.headers["Authorization"]!!
+                    if (user == "user-a") {
+                        // Simulate slower network for one of the requests
+                        delay(100.milliseconds)
+                    }
+                    val etag = "etag-of-$user"
+                    if (context.request.headers["If-None-Match"] == etag) {
+                        context.respond(HttpStatusCode.NotModified)
+                        return@get
+                    }
+                    context.response.header("Cache-Control", "no-cache")
+                    context.response.header("ETag", etag)
+                    context.respondText(user)
+                }
+            }
+        }
+
+        val client = createClient {
+            install(HttpCache)
+        }
+
+        assertEquals(
+            client.get("/me") {
+                headers["Authorization"] = "user-a"
+            }.bodyAsText(),
+            "user-a"
+        )
+        withContext(Dispatchers.Default) {
+            listOf(
+                launch {
+                    val response = client.get("/me") {
+                        headers["Authorization"] = "user-a"
+                    }.bodyAsText()
+
+                    assertEquals("user-b", response)
+                },
+                launch {
+                    val response = client.get("/me") {
+                        headers["Authorization"] = "user-b"
+                    }.bodyAsText()
+
+                    assertEquals("user-b", response)
+                }
+            ).joinAll()
+        }
+    }
+}