Skip to content

Commit 90cc359

Browse files
adimiz1adimiz1
authored
Fix API parameters signature
1 parent bae86bc commit 90cc359

File tree

7 files changed

+97
-27
lines changed

7 files changed

+97
-27
lines changed

‎cloudinary-core/src/main/java/com/cloudinary/Cloudinary.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,8 @@ public String signedPreloadedImage(Map result) {
130130
+ (result.containsKey("format") ? "." + result.get("format") : "") + "#" + result.get("signature");
131131
}
132132

133-
public String apiSignRequest(Map<String, Object> paramsToSign, String apiSecret) {
134-
return Util.produceSignature(paramsToSign, apiSecret, config.signatureAlgorithm);
133+
public String apiSignRequest(Map<String, Object> paramsToSign, String apiSecret, int signatureVersion) {
134+
return Util.produceSignature(paramsToSign, apiSecret, config.signatureAlgorithm, signatureVersion);
135135
}
136136

137137
/**
@@ -206,7 +206,7 @@ public void signRequest(Map<String, Object> params, Map<String, Object> options)
206206
if (apiSecret == null)
207207
throw new IllegalArgumentException("Must supply api_secret");
208208
Util.clearEmpty(params);
209-
params.put("signature", this.apiSignRequest(params, apiSecret));
209+
params.put("signature", this.apiSignRequest(params, apiSecret, this.config.signatureVersion));
210210
params.put("api_key", apiKey);
211211
}
212212

‎cloudinary-core/src/main/java/com/cloudinary/Configuration.java‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public class Configuration {
2121
public final static String USER_AGENT = "cld-android-" + VERSION;
2222
public static final boolean DEFAULT_IS_LONG_SIGNATURE = false;
2323
public static final SignatureAlgorithm DEFAULT_SIGNATURE_ALGORITHM = SignatureAlgorithm.SHA1;
24+
public static final int DEFAULT_SIGNATURE_VERSION = 2;
2425

2526
private static final String CONFIG_PROP_SIGNATURE_ALGORITHM = "signature_algorithm";
2627

@@ -48,6 +49,7 @@ public class Configuration {
4849
public boolean forceVersion = true;
4950
public boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE;
5051
public SignatureAlgorithm signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM;
52+
public int signatureVersion = DEFAULT_SIGNATURE_VERSION;
5153
public String oauthToken = null;
5254
public Boolean analytics;
5355
public Configuration() {
@@ -75,6 +77,7 @@ private Configuration(
7577
boolean forceVersion,
7678
boolean longUrlSignature,
7779
SignatureAlgorithm signatureAlgorithm,
80+
int signatureVersion,
7881
String oauthToken,
7982
boolean analytics) {
8083
this.cloudName = cloudName;
@@ -98,6 +101,7 @@ private Configuration(
98101
this.forceVersion = forceVersion;
99102
this.longUrlSignature = longUrlSignature;
100103
this.signatureAlgorithm = signatureAlgorithm;
104+
this.signatureVersion = signatureVersion;
101105
this.oauthToken = oauthToken;
102106
this.analytics = analytics;
103107
}
@@ -140,6 +144,7 @@ public void update(Map config) {
140144
}
141145
this.longUrlSignature = ObjectUtils.asBoolean(config.get("long_url_signature"), DEFAULT_IS_LONG_SIGNATURE);
142146
this.signatureAlgorithm = SignatureAlgorithm.valueOf(ObjectUtils.asString(config.get(CONFIG_PROP_SIGNATURE_ALGORITHM), DEFAULT_SIGNATURE_ALGORITHM.name()));
147+
this.signatureVersion = ObjectUtils.asInteger(config.get("signature_version"), DEFAULT_SIGNATURE_VERSION);
143148
this.oauthToken = (String) config.get("oauth_token");
144149

145150
}
@@ -173,6 +178,7 @@ public Map<String, Object> asMap() {
173178
map.put("properties", new HashMap<String,Object>(properties));
174179
map.put("long_url_signature", longUrlSignature);
175180
map.put(CONFIG_PROP_SIGNATURE_ALGORITHM, signatureAlgorithm.toString());
181+
map.put("signature_version", signatureVersion);
176182
map.put("oauth_token", oauthToken);
177183
map.put("analytics", analytics);
178184
return map;
@@ -206,6 +212,7 @@ public Configuration(Configuration other) {
206212
this.properties.putAll(other.properties);
207213
this.longUrlSignature = other.longUrlSignature;
208214
this.signatureAlgorithm = other.signatureAlgorithm;
215+
this.signatureVersion = other.signatureVersion;
209216
this.oauthToken = other.oauthToken;
210217
this.analytics = other.analytics;
211218
}
@@ -320,6 +327,7 @@ public static class Builder {
320327
private boolean forceVersion = true;
321328
private boolean longUrlSignature = DEFAULT_IS_LONG_SIGNATURE;
322329
private SignatureAlgorithm signatureAlgorithm = DEFAULT_SIGNATURE_ALGORITHM;
330+
private int signatureVersion = DEFAULT_SIGNATURE_VERSION;
323331
private String oauthToken = null;
324332
private boolean analytics;
325333

@@ -360,6 +368,7 @@ public Configuration build() {
360368
forceVersion,
361369
longUrlSignature,
362370
signatureAlgorithm,
371+
signatureVersion,
363372
oauthToken,
364373
analytics);
365374
configuration.clientHints = clientHints;
@@ -500,6 +509,11 @@ public Builder setSignatureAlgorithm(SignatureAlgorithm signatureAlgorithm) {
500509
return this;
501510
}
502511

512+
public Builder setSignatureVersion(int signatureVersion) {
513+
this.signatureVersion = signatureVersion;
514+
return this;
515+
}
516+
503517
public Builder setOAuthToken(String oauthToken) {
504518
this.oauthToken = oauthToken;
505519
return this;
@@ -535,6 +549,7 @@ public Builder from(Configuration other) {
535549
this.forceVersion = other.forceVersion;
536550
this.longUrlSignature = other.longUrlSignature;
537551
this.signatureAlgorithm = other.signatureAlgorithm;
552+
this.signatureVersion = other.signatureVersion;
538553
this.oauthToken = other.oauthToken;
539554
this.analytics = other.analytics;
540555
return this;

‎cloudinary-core/src/main/java/com/cloudinary/Util.java‎

Lines changed: 36 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -366,8 +366,8 @@ public static byte[] getUTF8Bytes(String string) {
366366
* @param apiSecret secret value
367367
* @return hex-string representation of signature calculated based on provided parameters map and secret
368368
*/
369-
public static String produceSignature(Map<String, Object> paramsToSign, String apiSecret) {
370-
return produceSignature(paramsToSign, apiSecret, SignatureAlgorithm.SHA1);
369+
public static String produceSignature(Map<String, Object> paramsToSign, String apiSecret, int signatureVersion) {
370+
return produceSignature(paramsToSign, apiSecret, SignatureAlgorithm.SHA1, signatureVersion);
371371
}
372372

373373
/**
@@ -384,22 +384,42 @@ public static String produceSignature(Map<String, Object> paramsToSign, String a
384384
* @param signatureAlgorithm type of hashing algorithm to use for calculation of HMAC
385385
* @return hex-string representation of signature calculated based on provided parameters map and secret
386386
*/
387-
public static String produceSignature(Map<String, Object> paramsToSign, String apiSecret, SignatureAlgorithm signatureAlgorithm) {
388-
Collection<String> params = new ArrayList<String>();
389-
for (Map.Entry<String, Object> param : new TreeMap<String, Object>(paramsToSign).entrySet()) {
390-
if (param.getValue() instanceof Collection) {
391-
params.add(param.getKey() + "=" + StringUtils.join((Collection) param.getValue(), ","));
392-
} else if (param.getValue() instanceof Object[]) {
393-
params.add(param.getKey() + "=" + StringUtils.join((Object[]) param.getValue(), ","));
394-
} else {
395-
if (StringUtils.isNotBlank(param.getValue())) {
396-
params.add(param.getKey() + "=" + param.getValue().toString());
397-
}
387+
public static String produceSignature(Map<String, Object> paramsToSign, String apiSecret, SignatureAlgorithm signatureAlgorithm, int signatureVersion) {
388+
Collection<String> flattenedParams = flattenAndSanitizeParams(paramsToSign, signatureVersion);
389+
String toSign = StringUtils.join(flattenedParams, "&") + apiSecret;
390+
byte[] hash = Util.hash(toSign, signatureAlgorithm);
391+
return StringUtils.encodeHexString(hash);
392+
}
393+
394+
private static Collection<String> flattenAndSanitizeParams(Map<String, Object> paramsToSign, int signatureVersion) {
395+
Collection<String> params = new ArrayList<>();
396+
397+
for (Map.Entry<String, Object> entry : new TreeMap<>(paramsToSign).entrySet()) {
398+
Object value = entry.getValue();
399+
String rawValue = null;
400+
401+
if (value instanceof Collection) {
402+
rawValue = StringUtils.join((Collection) value, ",");
403+
} else if (value instanceof Object[]) {
404+
rawValue = StringUtils.join((Object[]) value, ",");
405+
} else if (value != null && StringUtils.isNotBlank(value.toString())) {
406+
rawValue = value.toString();
407+
}
408+
409+
if (rawValue != null) {
410+
String sanitizedValue = (signatureVersion == 2)
411+
? escapeAmpersand(rawValue)
412+
: rawValue;
413+
414+
params.add(entry.getKey() + "=" + sanitizedValue);
398415
}
399416
}
400-
String to_sign = StringUtils.join(params, "&");
401-
byte[] hash = Util.hash(to_sign + apiSecret, signatureAlgorithm);
402-
return StringUtils.encodeHexString(hash);
417+
418+
return params;
419+
}
420+
421+
private static String escapeAmpersand(String input) {
422+
return input.replace("&", "%26");
403423
}
404424

405425
/**

‎cloudinary-core/src/main/java/com/cloudinary/api/signing/ApiResponseSignatureVerifier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,6 @@ public ApiResponseSignatureVerifier(String secretKey, SignatureAlgorithm signatu
6060
public boolean verifySignature(String publicId, String version, String signature) {
6161
return Util.produceSignature(ObjectUtils.asMap(
6262
"public_id", emptyIfNull(publicId),
63-
"version", emptyIfNull(version)), secretKey, signatureAlgorithm).equals(signature);
63+
"version", emptyIfNull(version)), secretKey, signatureAlgorithm, 1).equals(signature);
6464
}
6565
}

‎cloudinary-core/src/test/java/com/cloudinary/test/CloudinaryTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,14 +1438,14 @@ public void testCloudinaryUrlEmptyScheme() {
14381438
@Test
14391439
public void testApiSignRequestSHA1() {
14401440
cloudinary.config.signatureAlgorithm = SignatureAlgorithm.SHA1;
1441-
String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac");
1441+
String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac", cloudinary.config.signatureVersion);
14421442
assertEquals("14c00ba6d0dfdedbc86b316847d95b9e6cd46d94", signature);
14431443
}
14441444

14451445
@Test
14461446
public void testApiSignRequestSHA256() {
14471447
cloudinary.config.signatureAlgorithm = SignatureAlgorithm.SHA256;
1448-
String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac");
1448+
String signature = cloudinary.apiSignRequest(ObjectUtils.asMap("cloud_name", "dn6ot3ged", "timestamp", 1568810420, "username", "user@cloudinary.com"), "hdcixPpR2iKERPwqvH6sHdK9cyac", cloudinary.config.signatureVersion);
14491449
assertEquals("45ddaa4fa01f0c2826f32f669d2e4514faf275fe6df053f1a150e7beae58a3bd", signature);
14501450
}
14511451

‎cloudinary-test-common/src/main/java/com/cloudinary/test/AbstractApiTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1368,4 +1368,39 @@ public void testAllowDerivedNextCursor() throws Exception {
13681368
cloudinary.uploader().destroy(publicId, Collections.singletonMap("invalidate", true));
13691369
}
13701370
}
1371+
1372+
@Test
1373+
public void testSignatureWithEscapingCharacters() {
1374+
String API_SIGN_REQUEST_CLOUD_NAME = "dn6ot3ged";
1375+
String API_SIGN_REQUEST_TEST_SECRET = "hdcixPpR2iKERPwqvH6sHdK9cyac";
1376+
1377+
Map<String, Object> paramsWithAmpersand = new HashMap<>();
1378+
paramsWithAmpersand.put("cloud_name", API_SIGN_REQUEST_CLOUD_NAME);
1379+
paramsWithAmpersand.put("timestamp", 1568810420);
1380+
paramsWithAmpersand.put("notification_url", "https://fake.com/callback?a=1&tags=hello,world");
1381+
1382+
String signatureWithAmpersand = Util.produceSignature(paramsWithAmpersand, API_SIGN_REQUEST_TEST_SECRET, cloudinary.config.signatureVersion);
1383+
1384+
Map<String, Object> paramsSmuggled = new HashMap<>();
1385+
paramsSmuggled.put("cloud_name", API_SIGN_REQUEST_CLOUD_NAME);
1386+
paramsSmuggled.put("timestamp", 1568810420);
1387+
paramsSmuggled.put("notification_url", "https://fake.com/callback?a=1");
1388+
paramsSmuggled.put("tags", "hello,world");
1389+
1390+
String signatureSmuggled = Util.produceSignature(paramsSmuggled, API_SIGN_REQUEST_TEST_SECRET, cloudinary.config.signatureVersion);
1391+
1392+
assertNotEquals(signatureWithAmpersand, signatureSmuggled,
1393+
"Signatures should be different to prevent parameter smuggling");
1394+
1395+
String expectedSignature = "4fdf465dd89451cc1ed8ec5b3e314e8a51695704";
1396+
assertEquals(expectedSignature, signatureWithAmpersand);
1397+
1398+
String expectedSmuggledSignature = "7b4e3a539ff1fa6e6700c41b3a2ee77586a025f9";
1399+
assertEquals(expectedSmuggledSignature, signatureSmuggled);
1400+
1401+
String versionOneSignature = Util.produceSignature(paramsSmuggled, API_SIGN_REQUEST_TEST_SECRET, 1);
1402+
1403+
assertEquals(expectedSmuggledSignature, versionOneSignature);
1404+
1405+
}
13711406
}

‎cloudinary-test-common/src/main/java/com/cloudinary/test/AbstractUploaderTest.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public void testUtf8Upload() throws IOException {
104104
Map<String, Object> to_sign = new HashMap<String, Object>();
105105
to_sign.put("public_id", result.get("public_id"));
106106
to_sign.put("version", ObjectUtils.asString(result.get("version")));
107-
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret);
107+
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret, cloudinary.config.signatureVersion);
108108
assertEquals(result.get("signature"), expected_signature);
109109
}
110110

@@ -131,7 +131,7 @@ public void testUpload() throws IOException {
131131
Map<String, Object> to_sign = new HashMap<String, Object>();
132132
to_sign.put("public_id", result.get("public_id"));
133133
to_sign.put("version", ObjectUtils.asString(result.get("version")));
134-
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret);
134+
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret, cloudinary.config.signatureVersion);
135135
assertEquals(result.get("signature"), expected_signature);
136136
}
137137

@@ -167,7 +167,7 @@ public void testUploadUrl() throws IOException {
167167
Map<String, Object> to_sign = new HashMap<String, Object>();
168168
to_sign.put("public_id", result.get("public_id"));
169169
to_sign.put("version", ObjectUtils.asString(result.get("version")));
170-
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret);
170+
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret, cloudinary.config.signatureVersion);
171171
assertEquals(result.get("signature"), expected_signature);
172172
}
173173

@@ -179,7 +179,7 @@ public void testUploadLargeUrl() throws IOException {
179179
Map<String, Object> to_sign = new HashMap<String, Object>();
180180
to_sign.put("public_id", result.get("public_id"));
181181
to_sign.put("version", ObjectUtils.asString(result.get("version")));
182-
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret);
182+
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret, cloudinary.config.signatureVersion);
183183
assertEquals(result.get("signature"), expected_signature);
184184
}
185185

@@ -191,7 +191,7 @@ public void testUploadDataUri() throws IOException {
191191
Map<String, Object> to_sign = new HashMap<String, Object>();
192192
to_sign.put("public_id", result.get("public_id"));
193193
to_sign.put("version", ObjectUtils.asString(result.get("version")));
194-
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret);
194+
String expected_signature = cloudinary.apiSignRequest(to_sign, cloudinary.config.apiSecret, cloudinary.config.signatureVersion);
195195
assertEquals(result.get("signature"), expected_signature);
196196
}
197197

0 commit comments

Comments
 (0)