Serializing classes with properties/private properties can cause side effects

Already mentioned on #305, but creating a separate issue, as it's fixable independently of supporting class properties in full, and is a serious bug.

Problem

Input:

export default class C {
  foo = console.log('yo!');
};

This is instrumented as:

module.exports = class /*livepack_track:5;c;/src/index.js*/ C {
  foo = console.log('yo!');
  constructor() {
    livepack_tracker(livepack_getFnInfo_5, () => []);
  }
};

When Livepack attempts to serialize class C it calls new C(). This is intended to trigger the tracker in C's constructor, which records the class's info and throws an error to bail out before any user code in the constructor executes.

The problem in this case is that class properties are evaluated before the class constructor. So console.log('yo!') is executed.

This is a serious problem. The code which is executed could do anything, and in worst case could be a security vulnerability if an attacker was able to leverage it in order to execute code which is only meant to run on the client on a server at build time.

Solution

Solution is to put the tracker in the first class property, which will execute before anything else, instead of in the constructor:

module.exports = class /*livepack_track:5;c;/src/index.js*/ C {
  foo = (livepack_tracker(livepack_getFnInfo_5, () => []), console.log('yo!'));
};

This only applies to classes with no super class. If a class extends another class, the constructor executes before any class properties are evaluated.

Another (flawed) solution

Another solution would be to add a private property with the tracker before anything else:

module.exports = class /*livepack_track:5;c;/src/index.js*/ C {
  #livepack_dummy = livepack_tracker(livepack_getFnInfo_5, () => []);
  foo = console.log('yo!');
};

However, this approach has a tiny flaw. The name of the dummy private property can be selected so as not to clash with any existing private properties. It's a syntax error to use a non-existent private property elsewhere in the class e.g. class C { foo() { this.#bar } } and Babel parser catches this before instrumentation begins.

However, this can be defeated by eval():

class X {
  foo = 1;
  bar() {
    eval('this.#livepack_dummy');
  }
}

In original source, calling bar() would throw a syntax error. But if Livepack added a #livepack_dummy private property for the tracker, it would execute without error.

Adding tracker to an existing property does not suffer from this problem, so is preferable.

overlookmotel / livepack