Fix API parameters signature

Author: const-cloudinary

api/api.go

@@ -295,13 +295,33 @@ func BuildPath(parts ...interface{}) string {
 	return strings.Join(partsSlice, "/")
 }
 
-// SignParametersUsingAlgo signs parameters using the provided secret and sign algorithm.
-func SignParametersUsingAlgo(params url.Values, secret string, algo signature.Algo) (string, error) {
+// encodeParam encodes a parameter for safe inclusion in URL query strings.
+//
+// Specifically replaces "&" characters with their percent-encoded equivalent "%26"
+// to prevent them from being interpreted as parameter separators in URL query strings.
+// This encoding is only applied when signatureVersion is 2 or higher.
+func encodeParam(value string, signatureVersion int) string {
+	// Version 2: URL encode & characters in values to prevent parameter smuggling
+	if signatureVersion >= 2 {
+		return strings.Replace(value, "&", "%26", -1)
+	}
+	return value
+}
+
+// SignParametersUsingAlgoAndVersion signs parameters using the provided secret, sign algorithm, and signature version.
+func SignParametersUsingAlgoAndVersion(params url.Values, secret string, algo signature.Algo, signatureVersion int) (string, error) {
 	if _, withTimestamp := params["timestamp"]; !withTimestamp || params["timestamp"][0] == "0" {
 		params.Set("timestamp", strconv.FormatInt(time.Now().Unix(), 10))
 	}
 
-	encodedUnescapedParams, err := url.QueryUnescape(params.Encode())
+	encodedParams := make(url.Values)
+	for key, values := range params {
+		for _, value := range values {
+			encodedParams.Add(key, encodeParam(value, signatureVersion))
+		}
+	}
+
+	encodedUnescapedParams, err := url.QueryUnescape(encodedParams.Encode())
 	if err != nil {
 		return "", err
 	}
@@ -313,6 +333,11 @@ func SignParametersUsingAlgo(params url.Values, secret string, algo signature.Al
 	return hex.EncodeToString(rawSignature), nil
 }
 
+// SignParametersUsingAlgo signs parameters using the provided secret and sign algorithm.
+func SignParametersUsingAlgo(params url.Values, secret string, algo signature.Algo) (string, error) {
+	return SignParametersUsingAlgoAndVersion(params, secret, algo, 2)
+}
+
 // SignParameters signs parameters using the provided secret.
 func SignParameters(params url.Values, secret string) (string, error) {
 	return SignParametersUsingAlgo(params, secret, signature.SHA1)

api/api_test.go

@@ -0,0 +1,72 @@
+package api_test
+
+import (
+	"net/url"
+	"testing"
+
+	"github.com/cloudinary/cloudinary-go/v2/api"
+	"github.com/cloudinary/cloudinary-go/v2/internal/signature"
+	"github.com/stretchr/testify/assert"
+)
+
+// TestAPISignRequestPreventsParameterSmuggling tests that parameter smuggling via & characters is prevented.
+// Should prevent parameter smuggling via & characters in parameter values.
+func TestAPISignRequestPreventsParameterSmuggling(t *testing.T) {
+	const testSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+
+	// Test with notification_url containing & characters
+	paramsWithAmpersand := url.Values{}
+	paramsWithAmpersand.Set("cloud_name", "dn6ot3ged")
+	paramsWithAmpersand.Set("timestamp", "1568810420")
+	paramsWithAmpersand.Set("notification_url", "https://fake.com/callback?a=1&tags=hello,world")
+
+	signatureWithAmpersand, err := api.SignParametersUsingAlgo(paramsWithAmpersand, testSecret, signature.SHA1)
+	assert.NoError(t, err)
+
+	// Test that attempting to smuggle parameters by splitting the notification_url fails
+	paramsSmuggled := url.Values{}
+	paramsSmuggled.Set("cloud_name", "dn6ot3ged")
+	paramsSmuggled.Set("timestamp", "1568810420")
+	paramsSmuggled.Set("notification_url", "https://fake.com/callback?a=1")
+	paramsSmuggled.Set("tags", "hello,world") // This would be smuggled if & encoding didn't work
+
+	signatureSmuggled, err := api.SignParametersUsingAlgo(paramsSmuggled, testSecret, signature.SHA1)
+	assert.NoError(t, err)
+
+	// The signatures should be different, proving that parameter smuggling is prevented
+	assert.NotEqual(t, signatureWithAmpersand, signatureSmuggled,
+		"Signatures should be different to prevent parameter smuggling")
+
+	// Verify the expected signature for the properly encoded case
+	const expectedSignature = "4fdf465dd89451cc1ed8ec5b3e314e8a51695704"
+	assert.Equal(t, expectedSignature, signatureWithAmpersand)
+
+	// Verify the expected signature for the smuggled parameters case
+	const expectedSmuggledSignature = "7b4e3a539ff1fa6e6700c41b3a2ee77586a025f9"
+	assert.Equal(t, expectedSmuggledSignature, signatureSmuggled)
+}
+
+// TestConfiguredSignatureVersionIsApplied tests that the configured signature version is applied.
+func TestConfiguredSignatureVersionIsApplied(t *testing.T) {
+	const testSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+
+	params := url.Values{}
+	params.Set("cloud_name", "dn6ot3ged")
+	params.Set("timestamp", "1568810420")
+	params.Set("notification_url", "https://fake.com/callback?a=1&tags=hello,world")
+
+	// Test version 1 (no encoding)
+	signatureV1, err := api.SignParametersUsingAlgoAndVersion(params, testSecret, signature.SHA1, 1)
+	assert.NoError(t, err)
+
+	// Test version 2 (with encoding)
+	signatureV2, err := api.SignParametersUsingAlgoAndVersion(params, testSecret, signature.SHA1, 2)
+	assert.NoError(t, err)
+
+	// Should be different
+	assert.NotEqual(t, signatureV1, signatureV2, "Signatures should be different between versions")
+
+	// Version 2 should match expected signature from smuggling test
+	assert.Equal(t, "4fdf465dd89451cc1ed8ec5b3e314e8a51695704", signatureV2,
+		"Version 2 should match expected encoded signature")
+}

api/uploader/setup_test.go

@@ -0,0 +1,5 @@
+package uploader_test
+
+import "github.com/cloudinary/cloudinary-go/v2/api/uploader"
+
+var uploadAPI, _ = uploader.New()

api/uploader/upload.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/cloudinary/cloudinary-go/v2/api"
 	"github.com/cloudinary/cloudinary-go/v2/config"
+	"github.com/cloudinary/cloudinary-go/v2/internal/signature"
 	"github.com/google/uuid"
 
 	"github.com/cloudinary/cloudinary-go/v2/logger"
@@ -134,8 +135,8 @@ func (u *API) signRequest(requestParams url.Values) (url.Values, error) {
 		signatureParams[k] = []string{strings.Join(v, ",")}
 	}
 
-	signature, err := api.SignParametersUsingAlgo(signatureParams, u.Config.Cloud.APISecret,
-		u.Config.Cloud.GetSignatureAlgorithm())
+	signature, err := api.SignParametersUsingAlgoAndVersion(signatureParams, u.Config.Cloud.APISecret,
+		u.Config.Cloud.GetSignatureAlgorithm(), u.Config.Cloud.GetSignatureVersion())
 	if err != nil {
 		return nil, err
 	}
@@ -395,3 +396,43 @@ func min(a, b int64) int64 {
 	}
 	return b
 }
+
+// VerifyApiResponseSignature validates API response signature against Cloudinary configuration.
+// It validates that the response came from Cloudinary by checking the signature.
+func (u *API) VerifyApiResponseSignature(publicID string, version string, signature string) bool {
+	urlParams := make(url.Values)
+	urlParams.Set("public_id", publicID)
+	urlParams.Set("version", version)
+
+	// Use signature version 1 for API response validation (legacy behavior)
+	expectedSignature, err := api.SignParametersUsingAlgoAndVersion(urlParams, u.Config.Cloud.APISecret,
+		u.Config.Cloud.GetSignatureAlgorithm(), 1)
+	if err != nil {
+		return false
+	}
+
+	return signature == expectedSignature
+}
+
+// VerifyNotificationSignature validates notification signature against Cloudinary configuration.
+// It validates webhook notifications by checking the signature against the expected payload hash.
+func (u *API) VerifyNotificationSignature(body string, timestamp int64, receivedSignature string, validFor int64) bool {
+	if validFor <= 0 {
+		validFor = 7200 // Default: 2 hours
+	}
+
+	currentTimestamp := time.Now().Unix()
+	isSignatureExpired := timestamp <= currentTimestamp-validFor
+	if isSignatureExpired {
+		return false
+	}
+
+	payload := fmt.Sprintf("%s%d", body, timestamp)
+	rawSignature, err := signature.Sign(payload, u.Config.Cloud.APISecret, u.Config.Cloud.GetSignatureAlgorithm())
+	if err != nil {
+		return false
+	}
+	expectedSignature := hex.EncodeToString(rawSignature)
+
+	return receivedSignature == expectedSignature
+}

api/uploader/upload_asset_test.go

@@ -20,7 +20,6 @@ import (
 )
 
 var ctx = context.Background()
-var uploadAPI, _ = uploader.New()
 
 const largeImagePublicID = "go_test_large_image"
 const largeImageSize = 5880138

api/uploader/upload_test.go

@@ -0,0 +1,136 @@
+package uploader_test
+
+import (
+	"encoding/hex"
+	"fmt"
+	"net/url"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/cloudinary/cloudinary-go/v2/api"
+	"github.com/cloudinary/cloudinary-go/v2/api/uploader"
+	"github.com/cloudinary/cloudinary-go/v2/internal/signature"
+)
+
+// TestUploader_VerifyApiResponseSignature tests API response signature verification.
+func TestUploader_VerifyApiResponseSignature(t *testing.T) {
+	const publicID1 = "b8sjhoslj8cq8ovoa0ma"
+	const publicID2 = "z5sjhoskl2cq8ovoa0mv"
+	const version1 = "1555337587"
+	const version2 = "1555337588"
+
+	// Test valid signature
+	urlParams := make(url.Values)
+	urlParams.Set("public_id", publicID1)
+	urlParams.Set("version", version1)
+	correctSignature, err := api.SignParametersUsingAlgoAndVersion(urlParams, uploadAPI.Config.Cloud.APISecret,
+		uploadAPI.Config.Cloud.GetSignatureAlgorithm(), 1)
+	if err != nil {
+		t.Error(err)
+	}
+
+	isValid := uploadAPI.VerifyApiResponseSignature(publicID1, version1, correctSignature)
+	assert.True(t, isValid, "The response signature is valid for the same parameters")
+
+	// Test invalid signature with wrong version
+	urlParams.Set("version", version2)
+	newVersionSignature, err := api.SignParametersUsingAlgoAndVersion(urlParams, uploadAPI.Config.Cloud.APISecret,
+		uploadAPI.Config.Cloud.GetSignatureAlgorithm(), 1)
+	if err != nil {
+		t.Error(err)
+	}
+
+	isValid = uploadAPI.VerifyApiResponseSignature(publicID1, version1, newVersionSignature)
+	assert.False(t, isValid, "The response signature is invalid for the wrong version")
+
+	// Test invalid signature with wrong resource
+	urlParams.Set("version", version1)
+	urlParams.Set("public_id", publicID2)
+	anotherResourceSignature, err := api.SignParametersUsingAlgoAndVersion(urlParams, uploadAPI.Config.Cloud.APISecret,
+		uploadAPI.Config.Cloud.GetSignatureAlgorithm(), 1)
+	if err != nil {
+		t.Error(err)
+	}
+
+	isValid = uploadAPI.VerifyApiResponseSignature(publicID1, version1, anotherResourceSignature)
+	assert.False(t, isValid, "The response signature is invalid for the wrong resource")
+}
+
+// TestUploader_VerifyApiResponseSignatureWithAmpersand tests signature verification with & characters.
+func TestUploader_VerifyApiResponseSignatureWithAmpersand(t *testing.T) {
+	const testSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+
+	tempConfig := uploadAPI.Config
+	tempConfig.Cloud.APISecret = testSecret
+	tempAPI := &uploader.API{Config: tempConfig}
+	publicIDWithAmpersand := "callback?a=1&tags=hello,world"
+	version := "1568810420"
+
+	urlParams := make(url.Values)
+	urlParams.Set("public_id", publicIDWithAmpersand)
+	urlParams.Set("version", version)
+	v1Signature, err := api.SignParametersUsingAlgoAndVersion(urlParams, testSecret, signature.SHA1, 1)
+	if err != nil {
+		t.Error(err)
+	}
+	isValid := tempAPI.VerifyApiResponseSignature(publicIDWithAmpersand, version, v1Signature)
+	assert.True(t, isValid, "Should verify signature correctly with version 1")
+}
+
+// TestUploader_VerifyNotificationSignature tests webhook notification signature verification.
+func TestUploader_VerifyNotificationSignature(t *testing.T) {
+	const testSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+	body := `{"public_id":"b8sjhoslj8cq8ovoa0ma","version":"1555337587","width":"1000","height":"800"}`
+
+	tempConfig := uploadAPI.Config
+	tempConfig.Cloud.APISecret = testSecret
+	tempConfig.Cloud.SignatureAlgorithm = signature.SHA1
+	tempAPI := &uploader.API{Config: tempConfig}
+
+	currentTimestamp := time.Now().Unix()
+	validResponseTimestamp := currentTimestamp - 5000
+
+	payload := fmt.Sprintf("%s%d", body, validResponseTimestamp)
+	rawSignature, _ := signature.Sign(payload, testSecret, signature.SHA1)
+	validSignature := hex.EncodeToString(rawSignature)
+
+	// Test valid signature with sufficient time
+	isValid := tempAPI.VerifyNotificationSignature(body, validResponseTimestamp, validSignature, 7200)
+	assert.True(t, isValid, "The notification signature is valid for matching and not expired signature")
+
+	// Test expired signature
+	isValid = tempAPI.VerifyNotificationSignature(body, validResponseTimestamp, validSignature, 4000)
+	assert.False(t, isValid, "The notification signature is invalid for matching but expired signature")
+
+	// Test invalid signature
+	isValid = tempAPI.VerifyNotificationSignature(body, validResponseTimestamp, validSignature+"chars", 7200)
+	assert.False(t, isValid, "The notification signature is invalid for non matching and not expired signature")
+
+	// Test invalid signature with expiration
+	isValid = tempAPI.VerifyNotificationSignature(body, validResponseTimestamp, validSignature+"chars", 4000)
+	assert.False(t, isValid, "The notification signature is invalid for non matching and expired signature")
+}
+
+// TestUploader_VerifyNotificationSignatureWithSHA256 tests notification signature verification with SHA256.
+func TestUploader_VerifyNotificationSignatureWithSHA256(t *testing.T) {
+	const testSecret = "hdcixPpR2iKERPwqvH6sHdK9cyac"
+	body := `{}`
+
+	tempConfig := uploadAPI.Config
+	tempConfig.Cloud.APISecret = testSecret
+	tempConfig.Cloud.SignatureAlgorithm = signature.SHA256
+	tempAPI := &uploader.API{Config: tempConfig}
+
+	currentTimestamp := time.Now().Unix()
+	validResponseTimestamp := currentTimestamp - 5000
+
+	payload := fmt.Sprintf("%s%d", body, validResponseTimestamp)
+	rawSignature, _ := signature.Sign(payload, testSecret, signature.SHA256)
+	correctSignature := hex.EncodeToString(rawSignature)
+
+	// Test valid signature with SHA256 algorithm
+	isValid := tempAPI.VerifyNotificationSignature(body, validResponseTimestamp, correctSignature, 7200)
+	assert.True(t, isValid, "The notification signature is valid with SHA256")
+}

config/cloud.go

@@ -11,6 +11,7 @@ type Cloud struct {
 	APISecret          string `schema:"-"`
 	OAuthToken         string `schema:"oauth_token"`
 	SignatureAlgorithm string `schema:"signature_algorithm"`
+	SignatureVersion   int    `schema:"signature_version" default:"2"`
 }
 
 // GetSignatureAlgorithm returns the signature algorithm.
@@ -21,3 +22,12 @@ func (c Cloud) GetSignatureAlgorithm() string {
 
 	return c.SignatureAlgorithm
 }
+
+// GetSignatureVersion returns the signature version.
+func (c Cloud) GetSignatureVersion() int {
+	if c.SignatureVersion == 0 {
+		return 2
+	}
+
+	return c.SignatureVersion
+}

config/configuration_test.go

@@ -47,6 +47,7 @@ func TestConfiguration_CreateInstance(t *testing.T) {
 
 	// check a few default values
 	assert.EqualValues(t, signature.SHA1, c.Cloud.GetSignatureAlgorithm())
+	assert.EqualValues(t, 2, c.Cloud.GetSignatureVersion())
 	assert.EqualValues(t, 60, c.API.Timeout)
 	assert.EqualValues(t, true, c.URL.Secure)
 }
@@ -107,3 +108,21 @@ func TestConfiguration_AuthToken(t *testing.T) {
 	assert.EqualValues(t, 3, c.AuthToken.Expiration)
 	assert.EqualValues(t, 2, c.AuthToken.Duration)
 }
+
+func TestConfiguration_SignatureVersion(t *testing.T) {
+	c, err := config.NewFromURL(cldtest.CldURL + "?signature_version=1")
+	if err != nil {
+		t.Error("Error: ", err)
+	}
+
+	assert.Equal(t, cldtest.CloudName, c.Cloud.CloudName)
+	assert.EqualValues(t, 1, c.Cloud.GetSignatureVersion())
+
+	// Test default signature version
+	c2, err := config.NewFromURL(cldtest.CldURL)
+	if err != nil {
+		t.Error("Error: ", err)
+	}
+
+	assert.EqualValues(t, 2, c2.Cloud.GetSignatureVersion())
+}