odata: handle invalid skiptokens (getodk#1278)

alxndrsn · Dec 7, 2024 · 38fd622 · 38fd622
1 parent db45aef
commit 38fd622
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 3 deletions.
diff --git a/lib/formats/odata.js b/lib/formats/odata.js
@@ -513,7 +513,9 @@ const singleRowToOData = (fields, row, domain, originalUrl, query) => {
     let { offset } = paging;
 
     if (skipToken) {
-      offset = filtered.findIndex(s => skipToken.repeatId === s.__id) + 1;
+      const { repeatId } = skipToken;
+      if (!repeatId) throw Problem.user.odataInvalidSkiptoken();
+      offset = filtered.findIndex(s => repeatId === s.__id) + 1;
       if (offset === 0) throw Problem.user.odataRepeatIdNotFound();
     }
 

diff --git a/lib/util/db.js b/lib/util/db.js
@@ -379,8 +379,15 @@ class QueryOptions {
   //
   // See: https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html
   static parseSkiptoken(token) {
-    const jsonString = base64ToUtf8(token.substr(2));
-    return JSON.parse(jsonString);
+    if (!token.startsWith('01')) throw Problem.user.odataInvalidSkiptoken();
+
+    try {
+      const parsed = JSON.parse(base64ToUtf8(token.substr(2)));
+      if (typeof parsed !== 'object') throw Problem.user.odataInvalidSkiptoken();
+      return parsed;
+    } catch (err) {
+      throw Problem.user.odataInvalidSkiptoken();
+    }
   }
 
   static getSkiptoken(data) {

diff --git a/lib/util/problem.js b/lib/util/problem.js
@@ -134,6 +134,8 @@ const problems = {
 
     odataRepeatIdNotFound: problem(400.34, () => 'Record associated with the provided $skiptoken not found.'),
 
+    odataInvalidSkiptoken: problem(400.35, () => 'Invalid $skiptoken'),
+
     // no detail information for security reasons.
     authenticationFailed: problem(401.2, () => 'Could not authenticate with the provided credentials.'),
 

diff --git a/test/unit/formats/odata.js b/test/unit/formats/odata.js
@@ -979,6 +979,40 @@ describe('odata message composition', () => {
 
         const nomatch = '0000000000000000000000000000000000000000';
 
+        const stringify64 = obj => Buffer.from(JSON.stringify(obj)).toString('base64');
+
+        [
+          'nonsense',
+
+          // no version + valid token
+          stringify64({ repeatId: billy.__id }),
+
+          // incorrect version number + valid token
+          '00' + stringify64({ repeatId: billy.__id }),
+          '02' + stringify64({ repeatId: billy.__id }),
+
+          // correct version plus non-json
+          '01',
+          '01aGk=',
+
+          // correct version + empty JSON:
+          '01' + stringify64({}),
+          '01' + stringify64(''),
+
+          // correct version + non-base64 string
+          '01~',
+        ].forEach($skiptoken => {
+          it(`should throw error for malformed $skiptoken '${$skiptoken}'`, () =>
+            fieldsFor(testData.forms.withrepeat)
+              .then((fields) => {
+                const submission = mockSubmission('two', testData.instances.withrepeat.two);
+                const query = { $skiptoken };
+                const originaUrl = "/withrepeat.svc/Submissions('two')/children/child"; // doesn't have to include query string
+                return singleRowToOData(fields, submission, 'http://localhost:8989', originaUrl, query);
+              })
+              .should.be.rejectedWith(Problem, { problemCode: 400.35, message: 'Invalid $skiptoken' }));
+        });
+
         [
           {
             $top: 0,