Fix API parameters signature

Author: adimiz1

uploader/src/main/kotlin/com/cloudinary/upload/Uploader.kt

@@ -189,7 +189,7 @@ class Uploader internal constructor(val cloudinary: Cloudinary, clientFactory: H
                 paramsMap["timestamp"] = paramsMap["timestamp"]
                     ?: (System.currentTimeMillis() / 1000L).asCloudinaryTimestamp()
                 paramsMap["signature"] =
-                    paramsMap["signature"] ?: apiSignRequest(paramsMap, apiSecret)
+                    paramsMap["signature"] ?: apiSignRequest(paramsMap, apiSecret, config.urlConfig.signatureVersion)
                 paramsMap["api_key"] = apiKey
             }
 

uploader/src/main/kotlin/com/cloudinary/util/utils.kt

@@ -17,19 +17,29 @@ fun randomPublicId(): String {
     return bytes.toHex()
 }
 
-fun apiSignRequest(paramsToSign: MutableMap<String, Any>, apiSecret: String): String {
-    val params = ArrayList<String>()
-    paramsToSign.entries.forEach {
-        val value = if (it.value is List<*>)
-            (it.value as Collection<*>).joinToString(",")
-        else
-            it.value
-
-        params.add("${it.key}=${value}")
-    }
+fun apiSignRequest(paramsToSign: Map<String, Any>, apiSecret: String, signatureVersion: Int): String {
+    val queryString = paramsToSign.entries
+        .map {
+            val valueStr = if (it.value is List<*>) {
+                (it.value as Collection<*>).joinToString(",")
+            } else {
+                it.value.toString()
+            }
+
+            val sanitizedValue = if (signatureVersion == 2) {
+                valueStr.replace("&", "%26")
+            } else {
+                valueStr
+            }
+
+            "${it.key}=$sanitizedValue"
+        }
+        .filter { it.isNotBlank() }
+        .sorted()
+        .joinToString("&")
 
     return MessageDigest.getInstance("SHA-1")
-        .digest((params.filter { it.isNotBlank() }.sorted().joinToString("&") + apiSecret).toByteArray(Charsets.UTF_8))
+        .digest((queryString + apiSecret).toByteArray(Charsets.UTF_8))
         .toHex()
 }
 

uploader/src/test/kotlin/com/cloudinary/UploaderTest.kt

@@ -35,6 +35,7 @@ import java.net.URL
 import java.text.SimpleDateFormat
 import java.util.*
 import kotlin.test.Test
+import kotlin.test.assertNotEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertNull
 import kotlin.test.assertTrue
@@ -1045,13 +1046,45 @@ class UploaderTest(networkLayer: NetworkLayer) {
         assertNotNull(result.playbackUrl)
     }
 
+    @Test
+    fun testSignatureWithEscapingCharacters() {
+        val cloudName = "dn6ot3ged"
+        val apiSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+
+        val paramsWithAmpersand = mapOf(
+            "cloud_name" to cloudName,
+            "timestamp" to 1568810420,
+            "notification_url" to "https://fake.com/callback?a=1&tags=hello,world"
+        )
+
+        val signatureWithAmpersand = apiSignRequest(paramsWithAmpersand, apiSecret, cloudinary.config.urlConfig.signatureVersion)
+
+        val paramsSmuggled = mapOf(
+            "cloud_name" to cloudName,
+            "timestamp" to 1568810420,
+            "notification_url" to "https://fake.com/callback?a=1",
+            "tags" to "hello,world"
+        )
+
+        val signatureSmuggled = apiSignRequest(paramsSmuggled, apiSecret, cloudinary.config.urlConfig.signatureVersion)
+
+        assertNotEquals(signatureWithAmpersand, signatureSmuggled,
+            "Signatures should be different to prevent parameter smuggling")
+
+        val expectedSignature = "4fdf465dd89451cc1ed8ec5b3e314e8a51695704"
+        assertEquals(expectedSignature, signatureWithAmpersand)
+
+        val expectedSmuggledSignature = "7b4e3a539ff1fa6e6700c41b3a2ee77586a025f9"
+        assertEquals(expectedSmuggledSignature, signatureSmuggled)
+    }
+
     private fun validateSignature(result: UploadResult) {
         val toSign: MutableMap<String, Any> = HashMap()
         toSign["public_id"] = result.publicId!!
         toSign["version"] = result.version.toString()
 
         val expectedSignature: String =
-            apiSignRequest(toSign, cloudinary.config.apiSecret!!)
+            apiSignRequest(toSign, cloudinary.config.apiSecret!!, cloudinary.config.urlConfig.signatureVersion)
         assertEquals(result.signature, expectedSignature)
     }
 

url-gen/src/main/kotlin/com/cloudinary/config/UrlConfig.kt

@@ -10,6 +10,7 @@ internal const val DEFAULT_SECURE_CDN_SUBDOMAIN = false
 internal const val DEFAULT_LONG_URL_SIGNATURE = false
 internal const val DEFAULT_SIGN_URL = false
 internal const val DEFAULT_SIGNATURE_ALGORITHM = "SHA-1"
+internal const val DEFAULT_SIGNATURE_VERSION = 2
 internal const val DEFAULT_ANALYTICS = true
 
 const val SIGN_URL = "sign_url"
@@ -25,6 +26,7 @@ const val USE_ROOT_PATH = "use_root_path"
 const val CNAME = "cname"
 const val SECURE = "secure"
 const val SIGNATURE_ALGORITHM = "signature_algorithm"
+const val SIGNATURE_VERSION = "signature_version"
 const val ANALYTICS = "analytics"
 
 interface IUrlConfig {
@@ -41,6 +43,7 @@ interface IUrlConfig {
     val secure: Boolean
     val forceVersion: Boolean
     val signatureAlgorithm: String?
+    val signatureVersion: Int
     val analytics: Boolean
 }
 
@@ -61,6 +64,7 @@ data class UrlConfig(
     override val secure: Boolean = DEFAULT_SECURE,
     override val forceVersion: Boolean = DEFAULT_FORCE_VERSION,
     override val signatureAlgorithm: String = DEFAULT_SIGNATURE_ALGORITHM,
+    override val signatureVersion: Int = DEFAULT_SIGNATURE_VERSION,
     override val analytics: Boolean = DEFAULT_ANALYTICS
 ) : IUrlConfig {
     constructor(params: Map<String, Any>) : this(
@@ -76,6 +80,7 @@ data class UrlConfig(
         secure = params.getBoolean(SECURE) ?: DEFAULT_SECURE,
         forceVersion = params.getBoolean(FORCE_VERSION) ?: DEFAULT_FORCE_VERSION,
         signatureAlgorithm = params[SIGNATURE_ALGORITHM]?.toString() ?: DEFAULT_SIGNATURE_ALGORITHM,
+        signatureVersion = params.getInt(SIGNATURE_VERSION) ?: DEFAULT_SIGNATURE_VERSION,
         analytics = params.getBoolean(ANALYTICS) ?: DEFAULT_ANALYTICS
     )
 }