Skip to content

Client applies ops from the server without the json0 path guard the server uses #721

Description

@alecgibson

Summary

lib/ot.js refuses to apply a json0 op whose path contains a segment inherited from Object.prototype (__proto__, constructor, toString, …) — see applyOpEdit() and util.isDangerousProperty(). lib/client/doc.js has no equivalent check: _otApply() hands op data straight to this.type.apply(), so ops arriving from the server are applied unvalidated.

The server-side guard therefore protects the server's own Object.prototype, but nothing protects a client's. That matters in two situations:

  1. Documents whose history predates the guard. The guard landed in v4.1.2 (06cc387). A document created before that can have an op with a __proto__ path committed to the database. Any client that fetches or subscribes to that document replays the op locally and pollutes its own Object.prototype.
  2. A hostile or compromised server, or anything else that can put a frame on the client's socket.

The write is silent: doc.data is unchanged, no error is emitted, and nothing in the client notices.

Repro

No dependencies beyond sharedb's own. Save as repro-client-op.js in the repo root and node repro-client-op.js:

const Backend = require('./lib/backend');

const backend = new Backend();
const connection = backend.connect();
const doc = connection.get('c', 'd');

doc.create({n: 0}, (error) => {
  if (error) throw error;

  // Exactly what a hostile or compromised server can put on the wire. The doc is
  // subscribed and at v1, so the client accepts and applies this without question.
  connection.handleMessage({
    a: 'op',
    c: 'c',
    d: 'd',
    v: 1,
    src: 'hostile',
    seq: 1,
    op: [{p: ['__proto__', 'polluted'], oi: 'yes'}]
  });

  console.log('doc.data                 =', JSON.stringify(doc.data));
  console.log('({}).polluted            =', ({}).polluted);
  console.log('Object.prototype tainted =', 'polluted' in {});
});

Actual

doc.data                 = {"n":0}
({}).polluted            = yes
Object.prototype tainted = true

Expected

The op is rejected, Object.prototype is untouched, and the client surfaces the error the way it does for any other op it cannot apply.

Notes

  • Doc.prototype._otApply is called for remote ops (_handleOp), for the local submitOp path, for fixup ops echoed back by the server, and for the inverted op on rollback — so a single check inside _otApply would cover all of them. Its own comment already says "all usage of _otApply should be wrapped with a try/catch", and every call site does (each one hard-rollbacks), so throwing from a client-side check fits the existing contract.
  • this.type.apply() will not throw on a dangerous path — it applies it — so the check has to be explicit rather than relying on the type to complain.
  • The guard in lib/ot.js is json0-specific by design (it reads component.p), so any client-side equivalent would be too.
  • Fixing this does not clean up documents that already have such an op in their history; those ops stay in the database and keep failing (or, today, keep polluting) on every replay.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions