Skip to content

Fix API parameters signature #422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
const-cloudinary merged 2 commits into from
Jun 17, 2025
Merged

Fix API parameters signature #422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1c2ba09
Fix API parameters signature
const-cloudinary Jun 15, 2025
2c96380
Add `signatureVersion` config
const-cloudinary Jun 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/Api/Utils/ApiUtils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,8 +242,16 @@ public static function serializeResponsiveBreakpoints(?array $breakpoints): bool
*
* @internal
*/
public static function serializeQueryParams(array $parameters = []): string
public static function serializeQueryParams(array $parameters = [], int $signatureVersion = 2): string
{
// Version 2: URL encode & characters in values to prevent parameter smuggling
if ($signatureVersion >= 2) {
$parameters = ArrayUtils::mapAssoc(
static fn($key, $value) => str_replace('&', '%26', $value),
$parameters
);
}

return ArrayUtils::implodeAssoc(
$parameters,
self::QUERY_STRING_OUTER_DELIMITER,
Expand All @@ -257,6 +265,7 @@ public static function serializeQueryParams(array $parameters = []): string
* @param array $parameters Parameters to sign.
* @param string $secret The API secret of the cloud.
* @param string $signatureAlgorithm Signature algorithm
* @param int $signatureVersion Signature version (1 or 2)
*
* @return string The signature.
*
Expand All @@ -265,13 +274,14 @@ public static function serializeQueryParams(array $parameters = []): string
public static function signParameters(
array $parameters,
string $secret,
string $signatureAlgorithm = Utils::ALGO_SHA1
string $signatureAlgorithm = Utils::ALGO_SHA1,
int $signatureVersion = 2
): string {
$parameters = array_map(self::class . '::serializeSimpleApiParam', $parameters);

ksort($parameters);

$signatureContent = self::serializeQueryParams($parameters);
$signatureContent = self::serializeQueryParams($parameters, $signatureVersion);

return Utils::sign($signatureContent, $secret, false, $signatureAlgorithm);
}
Expand All @@ -287,7 +297,8 @@ public static function signRequest(?array &$parameters, CloudConfig $cloudConfig
$parameters['signature'] = self::signParameters(
$parameters,
$cloudConfig->apiSecret,
$cloudConfig->signatureAlgorithm
$cloudConfig->signatureAlgorithm,
$cloudConfig->signatureVersion
);
$parameters['api_key'] = $cloudConfig->apiKey;
}
Expand Down
8 changes: 8 additions & 0 deletions src/Configuration/CloudConfig.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* target="_blank">Get account details from the Cloudinary Console.</a>
*
* @property ?string $signatureAlgorithm By default, set to self::DEFAULT_SIGNATURE_ALGORITHM.
* @property ?int $signatureVersion By default, set to self::DEFAULT_SIGNATURE_VERSION.
*
* @api
*/
Expand All @@ -29,13 +30,15 @@ class CloudConfig extends BaseConfigSection
public const CONFIG_NAME = 'cloud';

public const DEFAULT_SIGNATURE_ALGORITHM = Utils::ALGO_SHA1;
public const DEFAULT_SIGNATURE_VERSION = 2;

// Supported parameters
public const CLOUD_NAME = 'cloud_name';
public const API_KEY = 'api_key';
public const API_SECRET = 'api_secret';
public const OAUTH_TOKEN = 'oauth_token';
public const SIGNATURE_ALGORITHM = 'signature_algorithm';
public const SIGNATURE_VERSION = 'signature_version';

/**
* @var array of configuration keys that contain sensitive data that should not be exported (for example api key)
Expand Down Expand Up @@ -69,6 +72,11 @@ class CloudConfig extends BaseConfigSection
*/
protected ?string $signatureAlgorithm = null;

/**
* Sets the signature version (2 by default).
*/
protected ?int $signatureVersion = null;

/**
* Serialises configuration section to a string representation.
*
Expand Down
14 changes: 14 additions & 0 deletions src/Configuration/CloudConfigTrait.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,20 @@ public function signatureAlgorithm(string $signatureAlgorithm): static
return $this->setCloudConfig(CloudConfig::SIGNATURE_ALGORITHM, $signatureAlgorithm);
}

/**
* Sets the signature version.
*
* @param int $signatureVersion The signature version to use. (Can be 1 or 2).
*
* @return $this
*
* @api
*/
public function signatureVersion(int $signatureVersion): static
{
return $this->setCloudConfig(CloudConfig::SIGNATURE_VERSION, $signatureVersion);
}

/**
* Sets the Cloud configuration key with the specified value.
*
Expand Down
94 changes: 86 additions & 8 deletions tests/Unit/Utils/ApiUtilsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
*/
final class ApiUtilsTest extends UnitTestCase
{
public function tearDown()
public const API_SIGN_REQUEST_TEST_SECRET = 'hdcixPpR2iKERPwqvH6sHdK9cyac';
public const API_SIGN_REQUEST_CLOUD_NAME = 'dn6ot3ged';

public function tearDown(): void
{
parent::tearDown();

Expand Down Expand Up @@ -230,7 +233,7 @@ public function dataProviderSignParameters()
],
[
'value' => ['p1' => 'v1=v2*|}{ & !@#$%^&*()_;/.,?><\\/|_+a'],
'result' => 'bbdc631f4b490c0ba65722d8dbf9300d1fd98e86',
'result' => 'ced1e363d8db0a8d7ebcfb9e67fadbf5ee78a0f1',
],
];
}
Expand Down Expand Up @@ -276,12 +279,12 @@ public function dataProviderSignWithAlgorithmParameters()
],
[
'value' => ['p1' => 'v1=v2*|}{ & !@#$%^&*()_;/.,?><\\/|_+a'],
'result' => 'bbdc631f4b490c0ba65722d8dbf9300d1fd98e86',
'result' => 'ced1e363d8db0a8d7ebcfb9e67fadbf5ee78a0f1',
'signatureAlgorithm' => 'sha1',
],
[
'value' => ['p1' => 'v1=v2*|}{ & !@#$%^&*()_;/.,?><\\/|_+a'],
'result' => '9cdbdd04f587b41db72d66437f6dac2a379cd899c0cf3c3430925b1beca6052d',
'result' => '0c06416c30bfc727eb2cbc9f93245be70bd6567c788b5bd93a3772e8253312bf',
'signatureAlgorithm' => 'sha256',
],
];
Expand Down Expand Up @@ -310,13 +313,13 @@ public function testSignParametersWithExplicitSignatureAlgorithm($value, $result
public function testApiSignRequestWithGlobalConfig()
{
$initialParams = [
'cloud_name' => 'dn6ot3ged',
'cloud_name' => self::API_SIGN_REQUEST_CLOUD_NAME,
'timestamp' => 1568810420,
'username' => 'user@cloudinary.com'
];

$params = $initialParams;
Configuration::instance()->cloud->apiSecret = 'hdcixPpR2iKERPwqvH6sHdK9cyac';
Configuration::instance()->cloud->apiSecret = self::API_SIGN_REQUEST_TEST_SECRET;
Configuration::instance()->cloud->signatureAlgorithm = Utils::ALGO_SHA256;
ApiUtils::signRequest($params, Configuration::instance()->cloud);
$expected = '45ddaa4fa01f0c2826f32f669d2e4514faf275fe6df053f1a150e7beae58a3bd';
Expand All @@ -335,15 +338,90 @@ public function testApiSignRequestWithGlobalConfig()
public function testApiSignRequestWithExplicitConfig()
{
$params = [
'cloud_name' => 'dn6ot3ged',
'cloud_name' => self::API_SIGN_REQUEST_CLOUD_NAME,
'timestamp' => 1568810420,
'username' => 'user@cloudinary.com'
];

$config = new Configuration('cloudinary://key:hdcixPpR2iKERPwqvH6sHdK9cyac@test123');
$config = new Configuration('cloudinary://key:' . self::API_SIGN_REQUEST_TEST_SECRET . '@test123');
$config->cloud->signatureAlgorithm = Utils::ALGO_SHA256;
ApiUtils::signRequest($params, $config->cloud);
$expected = '45ddaa4fa01f0c2826f32f669d2e4514faf275fe6df053f1a150e7beae58a3bd';
self::assertEquals($expected, $params['signature']);
}

/**
* Should prevent parameter smuggling via & characters in parameter values.
*/
public function testApiSignRequestPreventsParameterSmuggling()
{
// Test with notification_url containing & characters
$paramsWithAmpersand = [
'cloud_name' => self::API_SIGN_REQUEST_CLOUD_NAME,
'timestamp' => 1568810420,
'notification_url' => 'https://fake.com/callback?a=1&tags=hello,world'
];

$config = new Configuration('cloudinary://key:' . self::API_SIGN_REQUEST_TEST_SECRET . '@test123');
ApiUtils::signRequest($paramsWithAmpersand, $config->cloud);
$signatureWithAmpersand = $paramsWithAmpersand['signature'];

// Test that attempting to smuggle parameters by splitting the notification_url fails
$paramsSmugggled = [
'cloud_name' => self::API_SIGN_REQUEST_CLOUD_NAME,
'timestamp' => 1568810420,
'notification_url' => 'https://fake.com/callback?a=1',
'tags' => 'hello,world' // This would be smuggled if & encoding didn't work
];

ApiUtils::signRequest($paramsSmugggled, $config->cloud);
$signatureSmugggled = $paramsSmugggled['signature'];

// The signatures should be different, proving that parameter smuggling is prevented
self::assertNotEquals($signatureWithAmpersand, $signatureSmugggled,
'Signatures should be different to prevent parameter smuggling');

// Verify the expected signature for the properly encoded case
$expectedSignature = '4fdf465dd89451cc1ed8ec5b3e314e8a51695704';
self::assertEquals($expectedSignature, $signatureWithAmpersand);

// Verify the expected signature for the smuggled parameters case
$expectedSmuggledSignature = '7b4e3a539ff1fa6e6700c41b3a2ee77586a025f9';
self::assertEquals($expectedSmuggledSignature, $signatureSmugggled);
}

/**
* Should apply the configured signature version from CloudConfig.
*/
public function testConfiguredSignatureVersionIsApplied()
{
$params = [
'cloud_name' => self::API_SIGN_REQUEST_CLOUD_NAME,
'timestamp' => 1568810420,
'notification_url' => 'https://fake.com/callback?a=1&tags=hello,world'
];

$config = new Configuration('cloudinary://key:' . self::API_SIGN_REQUEST_TEST_SECRET . '@test123');

// Test with signature version 1 (legacy behavior - no URL encoding)
$config->cloud->signatureVersion = 1;
$paramsV1 = $params;
ApiUtils::signRequest($paramsV1, $config->cloud);
$signatureV1 = $paramsV1['signature'];

// Test with signature version 2 (current behavior - with URL encoding)
$config->cloud->signatureVersion = 2;
$paramsV2 = $params;
ApiUtils::signRequest($paramsV2, $config->cloud);
$signatureV2 = $paramsV2['signature'];

// Signatures should be different, proving the version setting is applied
self::assertNotEquals($signatureV1, $signatureV2,
'Signature versions should produce different results');

// Version 2 should match the expected encoded signature
$expectedV2Signature = '4fdf465dd89451cc1ed8ec5b3e314e8a51695704';
self::assertEquals($expectedV2Signature, $signatureV2,
'Version 2 should match expected encoded signature');
}
}
Loading