validateRideLength accepts any Object.prototype key as a ride length
Nobody has claimed this yet.
Assessment
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Newbie friendliness
- 78/100
- Issue type
- Bug
- Clarity
- Clearly specified
- Activity status
- Quiet
- Tech stack
- javascript
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.
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.
- Dominant language
- JavaScript
- Stars
- 30
- Forks
- 25
- Avg merge
- 9m
- Merged PRs (30d)
- 1
Getting set up
- Ships a Dockerfile or Docker Compose file
- No pull request template
- No contributing guide
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.
More from shift-org/shift-docs
-
Difficulty 1/5 Under an hour Newbie friendliness 88/100
shift-org/shift-docs#1091 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
shift-org/shift-docs#1087 ·
-
Difficulty 4/5 3-5 days Newbie friendliness 68/100
shift-org/shift-docs#1082 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 58/100
shift-org/shift-docs#1072 ·
-
Difficulty 3/5 1-2 days Newbie friendliness 58/100
shift-org/shift-docs#1054 ·
All issues in shift-org/shift-docs
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 85/100
Maintainers usually reply within 3 days
-
Add: Atlas TVOpenchannels:add check:passed
Difficulty 2/5 1-3 hours Newbie friendliness 74/100
Maintainers usually reply within 4 days
-
bug
Difficulty 2/5 1-3 hours Newbie friendliness 82/100
jaegertracing/jaeger-ui#4547 · 3 comments ·
Maintainers usually reply within 1 day
-
feedback simulation workshop
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
githubnext/gh-aw-workshop#4090 ·
Maintainers usually reply within 1 day
-
bug deck: add to staging level: missing p-feature: Manage Submissions p-feature: Submissions and process priority: MUST HAVE ready for dev lead role: missing size: missing time sensitive
Difficulty 2/5 1-3 hours Newbie friendliness 67/100
hackforla/tdm-calculator#3581 ·
Maintainers usually reply within 2 days