shift-org / shift-org/shift-docs
validateRideLength accepts any Object.prototype key as a ride length
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 30
- Forks
- 25
- Avg merge
- 9m
- Merged PRs (30d)
- 1
Description
validateRideLength uses the in operator to check a submitted ride length against the known values. in walks the prototype chain, so alongside the four real lengths it also accepts every property inherited from Object.prototype — toString, constructor, hasOwnProperty, valueOf, __proto__ — and returns them unchanged, with no validation error.
Those values reach the database and are rendered back on the event page.
Reproducing
From the app directory:
node -e "
const { makeValidator, ErrorCollector } = require('./models/calEventValidator');
for (const v of ['0-3', 'bogus', 'toString', 'constructor', '__proto__']) {
const errors = new ErrorCollector();
const got = makeValidator({ ridelength: v }, errors).validateRideLength('ridelength');
console.log(JSON.stringify(v).padEnd(15), '->', JSON.stringify(got));
}"
On main this prints:
"0-3" -> "0-3" (correct)
"bogus" -> null (correctly rejected)
"toString" -> "toString" (should be null)
"constructor" -> "constructor" (should be null)
"__proto__" -> "__proto__" (should be null)
hasOwnProperty and valueOf behave the same way. None of them record a validation error.
It reaches the database and the page
Posting an otherwise valid event to manage_event with ridelength: "constructor" (payload shape per CALENDAR_API.md) returns 200, and reading the event back through retrieve_event confirms it stored:
ridelength: "constructor"
EventDetails.vue renders the field directly:
rideLength() {
if (this.evt.ridelength) {
return `${this.evt.ridelength} miles`;
}
so the event details page then displays "constructor miles".
Cause
app/models/calEventValidator.js:
validateRideLength(rideLength) {
value = getString(rideLength);
return (value in RideLength) ? value : null;
}
RideLength is an ordinary frozen object, so it still inherits from Object.prototype, and in finds those inherited keys.
A second, smaller problem in the same function
value is assigned without const/let, so it becomes an implicit global on every call — confirmed with globalThis.value === "0-3" after one call. It is harmless today only because this file is non-strict CommonJS; it would throw a ReferenceError under ESM or "use strict", which is where the rest of the project is heading.
Suggested fix
Use an own-property check, and declare the local:
- validateRideLength(rideLength) {
- value = getString(rideLength);
- return (value in RideLength) ? value : null;
- },
+ validateRideLength(field) {
+ const value = getString(field);
+ return Object.hasOwn(RideLength, value) ? value : null;
+ },
Severity
Low. The column is a varchar, and Vue escapes its output, so this is bad data rather than injection — an organizer can write arbitrary prototype-key strings into a public field on their own ride. Worth fixing because it is small, and because the same in-instead-of-own-property pattern is easy to copy into somewhere it matters more.
Not changed
Unlike the other validators, this one silently returns null for an unrecognised value rather than recording an error via errors.addError(field). That may well be deliberate — rejecting a bad ride length outright would fail the whole save — so I have left the behaviour alone. Flagging it in case it should be revisited separately.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in app/models/calEventValidator.js at validateRideLength and run the node reproduction from the issue in the app directory. Check the own-property validation and local variable behavior against the listed real and prototype-key values. Done means valid ride lengths still pass, prototype keys are rejected, and no global value is created.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript
- Domain
- backend, database
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100