fix: evm_revert crash when passed invalid subscriptionIds (fixes #386)

lib/statemanager.js

@@ -807,22 +807,31 @@ StateManager.prototype.snapshot = function(callback) {
 };
 
 StateManager.prototype.revert = function(snapshotId, callback) {
-  var self = this;
-
+  if (snapshotId === null || snapshotId === undefined) {
+    callback(new Error("invalid snapshotId"));
+    return;
+  }
   // Convert from hex.
-  snapshotId = utils.bufferToInt(snapshotId);
+  try {
+    snapshotId = utils.bufferToInt(snapshotId);
+  } catch (e) {
+    callback(e);
+    return;
+  }
 
   this.logger.log("Reverting to snapshot #" + snapshotId);
 
-  if (snapshotId > this.snapshots.length) {
+  if (snapshotId > this.snapshots.length || snapshotId <= 0) {
     // the snapshot doesn't exist now, or it has already been reverted
     callback(null, false);
     return false;
   }
 
   // Convert to zero based.
   snapshotId = snapshotId - 1;
-  var timeAdjustment = this.snapshots[snapshotId].timeAdjustment;
+  const snapShot = this.snapshots[snapshotId];
+  var timeAdjustment = snapShot.timeAdjustment;
+  var self = this;
 
   // Loop through each snapshot with a higher id than the current one.
   async.whilst(

test/snapshotting.js

@@ -105,4 +105,29 @@ describe("Checkpointing / Reverting", function() {
     let n4 = await instance.methods.n().call();
     assert.strictEqual(n4, "43", "n is not 43 after calling `inc` again");
   });
+
+  it("evm_revert rejects invalid subscriptionId types without crashing", async() => {
+    const { send } = context;
+    const ids = [{ foo: "bar" }, true, false];
+    await Promise.all(
+      ids.map((id) => assert.rejects(send("evm_revert", id), /invalid type/, "evm_revert did not reject as expected"))
+    );
+  });
+
+  it("evm_revert rejects null/undefined subscriptionId values", async() => {
+    const { send } = context;
+    const ids = [null, undefined];
+    await Promise.all(
+      ids.map((id) =>
+        assert.rejects(send("evm_revert", id), /invalid snapshotId/, "evm_revert did not reject as expected")
+      )
+    );
+  });
+
+  it("evm_revert returns false for out-of-range subscriptionId values", async() => {
+    const { send } = context;
+    const ids = [-1, Infinity, -Infinity, Buffer.from([0]), 0.5];
+    const promises = ids.map((id) => send("evm_revert", id).then((r) => assert(r, false)));
+    await Promise.all(promises);
+  });
 });