ImportEtherpad: Rigorously check imported data

Author: rhansen

src/node/db/PadManager.js

@@ -20,7 +20,7 @@
  */
 
 const CustomError = require('../utils/customError');
-const Pad = require('../db/Pad').Pad;
+const Pad = require('../db/Pad');
 const db = require('./DB');
 const hooks = require('../../static/js/pluginfw/hooks');
 
@@ -138,7 +138,7 @@ exports.getPad = async (id, text) => {
   }
 
   // try to load pad
-  pad = new Pad(id);
+  pad = new Pad.Pad(id);
 
   // initialize the pad
   await pad.init(text);

src/node/utils/ImportEtherpad.js

@@ -16,6 +16,7 @@
  * limitations under the License.
  */
 
+const {Pad} = require('../db/Pad');
 const authorManager = require('../db/AuthorManager');
 const db = require('../db/DB');
 const hooks = require('../../static/js/pluginfw/hooks');
@@ -90,6 +91,21 @@ exports.setPadRaw = async (padId, r) => {
     dbRecords.set(key, value);
   }));
 
+  const pad = new Pad(padId, {
+    // Only fetchers are needed to check the pad's integrity.
+    get: async (k) => dbRecords.get(k),
+    getSub: async (k, sub) => {
+      let v = dbRecords.get(k);
+      for (const sk of sub) {
+        if (v == null) return null;
+        v = v[sk];
+      }
+      return v;
+    },
+  });
+  await pad.init();
+  await pad.check();
+
   await Promise.all([
     ...[...dbRecords].map(async ([k, v]) => await db.set(k, v)),
     ...[...existingAuthors].map(async (authorId) => await authorManager.addPad(authorId, padId)),

src/tests/backend/specs/api/importexportGetPost.js

@@ -315,6 +315,118 @@ describe(__filename, function () {
           });
     });
 
+    describe('malformed .etherpad files are rejected', function () {
+      const makeGoodExport = () => ({
+        'pad:testing': {
+          atext: {
+            text: 'foo\n',
+            attribs: '|1+4',
+          },
+          pool: {
+            numToAttrib: {
+              0: ['author', 'a.foo'],
+            },
+            nextNum: 1,
+          },
+          head: 0,
+          savedRevisions: [],
+        },
+        'globalAuthor:a.foo': {
+          colorId: '#000000',
+          name: 'author foo',
+          timestamp: 1598747784631,
+          padIDs: 'testing',
+        },
+        'pad:testing:revs:0': {
+          changeset: 'Z:1>3+3$foo',
+          meta: {
+            author: 'a.foo',
+            timestamp: 1597632398288,
+            pool: {
+              numToAttrib: {},
+              nextNum: 0,
+            },
+            atext: {
+              text: 'foo\n',
+              attribs: '|1+4',
+            },
+          },
+        },
+      });
+
+      const importEtherpad = (records) => agent.post(`/p/${testPadId}/import`)
+          .attach('file', Buffer.from(JSON.stringify(records), 'utf8'), {
+            filename: '/test.etherpad',
+            contentType: 'application/etherpad',
+          });
+
+      before(async function () {
+        // makeGoodExport() is assumed to produce good .etherpad records. Verify that assumption so
+        // that a buggy makeGoodExport() doesn't cause checks to accidentally pass.
+        const records = makeGoodExport();
+        await importEtherpad(records)
+            .expect(200)
+            .expect('Content-Type', /json/)
+            .expect((res) => assert.deepEqual(res.body, {
+              code: 0,
+              message: 'ok',
+              data: {directDatabaseAccess: true},
+            }));
+        await agent.get(`/p/${testPadId}/export/txt`)
+            .expect(200)
+            .buffer(true).parse(superagent.parse.text)
+            .expect((res) => assert.match(res.text, /foo/));
+      });
+
+      it('missing rev', async function () {
+        const records = makeGoodExport();
+        delete records['pad:testing:revs:0'];
+        await importEtherpad(records).expect(500);
+      });
+
+      it('bad changeset', async function () {
+        const records = makeGoodExport();
+        records['pad:testing:revs:0'].changeset = 'garbage';
+        await importEtherpad(records).expect(500);
+      });
+
+      it('missing attrib in pool', async function () {
+        const records = makeGoodExport();
+        records['pad:testing'].pool.nextNum++;
+        await importEtherpad(records).expect(500);
+      });
+
+      it('extra attrib in pool', async function () {
+        const records = makeGoodExport();
+        const pool = records['pad:testing'].pool;
+        pool.numToAttrib[pool.nextNum] = ['key', 'value'];
+        await importEtherpad(records).expect(500);
+      });
+
+      it('changeset refers to non-existent attrib', async function () {
+        const records = makeGoodExport();
+        records['pad:testing:revs:1'] = {
+          changeset: 'Z:4>4*1+4$asdf',
+          meta: {
+            author: 'a.foo',
+            timestamp: 1597632398288,
+          },
+        };
+        records['pad:testing'].head = 1;
+        records['pad:testing'].atext = {
+          text: 'asdffoo\n',
+          attribs: '*1+4|1+4',
+        };
+        await importEtherpad(records).expect(500);
+      });
+
+      it('pad atext does not match', async function () {
+        const records = makeGoodExport();
+        records['pad:testing'].atext.attribs = `*0${records['pad:testing'].atext.attribs}`;
+        await importEtherpad(records).expect(500);
+      });
+    });
+
     describe('Import authorization checks', function () {
       let authorize;