Reject prototype keys in validateRideLength (#1089)

app/models/calEventValidator.js

@@ -187,9 +187,12 @@ function makeValidator(input, errors) {
       return validStatus;
     },
 
-    validateRideLength(rideLength) {
-      value = getString(rideLength);
-      return (value in RideLength) ? value : null;
+    // if not specified ( or not one of the known lengths ) returns null.
+    // note: uses hasOwn, not 'in': 'in' walks the prototype chain,
+    // which would accept "toString", "constructor", etc. as ride lengths.
+    validateRideLength(field) {
+      const value = getString(field);
+      return Object.hasOwn(RideLength, value) ? value : null;
     },
   };
 }

app/test/validator_test.js

@@ -1,4 +1,5 @@
 const { ErrorCollector, makeValidator } = require("../models/calEventValidator");
+const { RideLength } = require("../models/calConst");
 const { describe, it } = require("node:test");
 const assert = require("node:assert/strict");
 
@@ -48,4 +49,38 @@ describe('event field validation', () => {
     assert.ok(msg.key);
     assert.equal(msg.key, `Please enter a value for <span class="field-name">key</span>`);
   });
+  it('ride length validator should accept the known lengths', () => {
+    for (const want of Object.keys(RideLength)) {
+      const errors = new ErrorCollector();
+      const v = makeValidator({ ridelength: want }, errors);
+      assert.equal(v.validateRideLength('ridelength'), want);
+      assert.equal(errors.count, 0);
+    }
+  });
+  it('ride length validator should reject anything else', () => {
+    const list = [
+      "bogus",
+      "",
+      null,
+      // these are inherited from Object.prototype;
+      // an 'in' test would let them through. re: #1089
+      "toString",
+      "constructor",
+      "hasOwnProperty",
+      "valueOf",
+      "__proto__",
+    ];
+    for (const bad of list) {
+      const errors = new ErrorCollector();
+      const v = makeValidator({ ridelength: bad }, errors);
+      assert.equal(v.validateRideLength('ridelength'), null, `for ${JSON.stringify(bad)}`);
+    }
+  });
+  it('ride length validator should not leak a global', () => {
+    delete globalThis.value;
+    const errors = new ErrorCollector();
+    const v = makeValidator({ ridelength: '0-3' }, errors);
+    v.validateRideLength('ridelength');
+    assert.equal(globalThis.value, undefined, "expected no implicit global");
+  });
 });