Commits

Peter Brownlow  committed 99fb29e Merge

Merged in feature/ACDEV-1048-qsh-ambiguous-with-ampersands-in-path (pull request #52)

ACDEV-1048: encoding potentially ambiguous ampersands in path

  • Participants
  • Parent commits 1d889b3, 2938b97

Comments (0)

Files changed (3)

File lib/internal/jwt.js

 
 
 /**
+ * The separator between sections of a canonical query.
+ */
+var CANONICAL_QUERY_SEPARATOR = '&';
+
+
+/**
  * expose object
  */
 var jwt = module.exports;
 };
 
 jwt.createCanonicalRequest = function createCanonicalRequest(req, checkBodyForParams, addonBaseUrl) {
-    return canonicalizeMethod(req) + '&' + canonicalizeUri(req, addonBaseUrl) + '&' + canonicalizeQueryString(req, checkBodyForParams);
+    return canonicalizeMethod(req) + CANONICAL_QUERY_SEPARATOR + canonicalizeUri(req, addonBaseUrl) + CANONICAL_QUERY_SEPARATOR + canonicalizeQueryString(req, checkBodyForParams);
 };
 
 jwt.createQueryStringHash = function createQueryStringHash(req, checkBodyForParams, addonBaseUrl) {
         return '/';
     }
 
+    // If the separator is not URL encoded then the following URLs have the same query-string-hash:
+    //   https://djtest9.jira-dev.com/rest/api/2/project&a=b?x=y
+    //   https://djtest9.jira-dev.com/rest/api/2/project?a=b&x=y
+    path = path.replace(new RegExp(CANONICAL_QUERY_SEPARATOR, 'g'), encodeRfc3986(CANONICAL_QUERY_SEPARATOR));
+
     // prefix with /
     if (path[0] !== '/') {
         path = '/' + path;

File test/jwt_test.js

         done();
     });
 
+    // If the separator is not URL encoded then the following URLs have the same query-string-hash:
+    //   https://djtest9.jira-dev.com/rest/api/2/project&a=b?x=y
+    //   https://djtest9.jira-dev.com/rest/api/2/project?a=b&x=y
+    describe('paths containing "&" characters should not have spoof-able qsh claims', function () {
+
+        it('requests that differ by ampersands in the path versus query-string do not have the same canonical request string', function (done) {
+            var req1 = {
+                method: 'post',
+                path: '/rest/api/2/project&a=b',
+                query: qs.parse('x=y'),
+                body: ''
+            };
+            var req2 = {
+                method: 'post',
+                path: '/rest/api/2/project',
+                query: qs.parse('a=b&x=y'),
+                body: ''
+            };
+
+            assert.notEqual(jwt.createCanonicalRequest(req1, false, ''), jwt.createCanonicalRequest(req2, false, ''));
+            done();
+        });
+
+        it('an ampersand in the path is url-encoded', function (done) {
+            var req = {
+                method: 'post',
+                path: '/rest/api/2/project&a=b',
+                query: qs.parse('x=y'),
+                body: ''
+            };
+
+            assert.equal(jwt.createCanonicalRequest(req, false, ''), 'POST&/rest/api/2/project%26a=b&x=y');
+            done();
+        });
+
+        it('multiple ampersands in the path are encoded', function (done) {
+            var req = {
+                method: 'post',
+                path: '/rest/api/2/project&a=b&c=d',
+                query: qs.parse('x=y'),
+                body: ''
+            };
+
+            assert.equal(jwt.createCanonicalRequest(req, false, ''), 'POST&/rest/api/2/project%26a=b%26c=d&x=y');
+            done();
+        });
+    });
+
     it('should correctly create qsh without query string', function (done) {
 
         var req = {

File test/resources/jwt-signed-urls.json

     },
     {
         "name": "BasePath RFC3986 Subdelimiters",
-        "canonicalUrl": "GET\u0026/path!$\u0026\u0027()*+,;\u003d\u0026a\u003db",
-        "signedUrl": "https://example.com/path!$\u0026\u0027()*+,;\u003d?a\u003db\u0026jwt\u003deyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzOTIyMDQ2NDIsInN1YiI6IjEyMzQ1Njc4OSIsImlzcyI6ImppcmE6MTIzNC01Njc4LTkwMDAiLCJxc2giOiIyMGRjM2M4OGNhYzAyYmQwOTBiZjk4NjQ4NWU0NTgzNzFkODc1ZjUwZWY3NjA2M2Q2YTQwYmVlYWQ3M2FjYTliIiwiaWF0IjoxMzkyMjA0NDYyfQ.pxql5f3TFSAhgRIUS0zYr1MIlyJwB814KquBu2XXdUY"
+        "canonicalUrl": "GET\u0026/path!$%26\u0027()*+,;\u003d\u0026a\u003db",
+        "signedUrl": "https://example.com/path!$\u0026\u0027()*+,;\u003d?a\u003db\u0026jwt\u003deyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzOTg3NDk3MDgsInN1YiI6IjEyMzQ1Njc4OSIsImlzcyI6ImppcmE6MTIzNC01Njc4LTkwMDAiLCJxc2giOiI5ZWY4MDNmOWNiOGRlYTRmN2Y4NzVjYWZmOGQzMWU5NTk2MmM2ZThiZDQ0ZDY0YTg0OGQ2ZWJiMDU1YjIxNDRiIiwiaWF0IjoxMzk4NzQ5NTI4fQ.stOFE3Oe6cZqsP5BGaNEjIF-LVK_zRRLyNU1y2_lKe0"
     },
     {
         "name": "BasePath RFC3986 Subdelimiters (uri)",
-        "canonicalUrl": "GET\u0026/path!$\u0026\u0027()*+,;\u003d\u0026a\u003db",
-        "signedUrl": "https://example.com/path!$\u0026\u0027()*+,;\u003d?a\u003db\u0026jwt\u003deyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzOTIyMDQ2NDIsInN1YiI6IjEyMzQ1Njc4OSIsImlzcyI6ImppcmE6MTIzNC01Njc4LTkwMDAiLCJxc2giOiIyMGRjM2M4OGNhYzAyYmQwOTBiZjk4NjQ4NWU0NTgzNzFkODc1ZjUwZWY3NjA2M2Q2YTQwYmVlYWQ3M2FjYTliIiwiaWF0IjoxMzkyMjA0NDYyfQ.pxql5f3TFSAhgRIUS0zYr1MIlyJwB814KquBu2XXdUY"
+        "canonicalUrl": "GET\u0026/path!$%26\u0027()*+,;\u003d\u0026a\u003db",
+        "signedUrl": "https://example.com/path!$\u0026\u0027()*+,;\u003d?a\u003db\u0026jwt\u003deyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjEzOTg3NDk3MDgsInN1YiI6IjEyMzQ1Njc4OSIsImlzcyI6ImppcmE6MTIzNC01Njc4LTkwMDAiLCJxc2giOiI5ZWY4MDNmOWNiOGRlYTRmN2Y4NzVjYWZmOGQzMWU5NTk2MmM2ZThiZDQ0ZDY0YTg0OGQ2ZWJiMDU1YjIxNDRiIiwiaWF0IjoxMzk4NzQ5NTI4fQ.stOFE3Oe6cZqsP5BGaNEjIF-LVK_zRRLyNU1y2_lKe0"
     },
     {
         "name": "Add-on baseUrl contains path",