Skip to content

Commit f28c126

Browse files
committed
fix: 6 cache poisoning bugs + security hardening
Cache key fixes (query-do.ts, local-executor.ts): - computedColumns used cc.name (undefined) instead of cc.alias - setOperation used .type/.table (undefined) instead of .mode/.right - subqueryIn treated array as single object instead of iterating - distinct only hashed presence, not column names - windows missing partitionBy/orderBy/frame/args fields - join entirely absent from cache key - version missing from query-do.ts - Added \0 separators between all hash inputs to prevent concatenation ambiguity Security hardening: - Gate /upload endpoint behind DEV_MODE env var (worker.ts) - Add path traversal validation on table names (query-schema.ts) - Add DEV_MODE to Env interface (types.ts)
1 parent 97f9241 commit f28c126

5 files changed

Lines changed: 54 additions & 33 deletions

File tree

src/local-executor.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -367,30 +367,37 @@ export class LocalExecutor implements QueryExecutor {
367367
private queryCacheKey(query: QueryDescriptor): string {
368368
let h = 0x811c9dc5;
369369
const feed = (s: string) => { for (let i = 0; i < s.length; i++) { h ^= s.charCodeAt(i); h = Math.imul(h, 0x01000193); } };
370-
feed(query.table);
370+
feed(query.table); feed("\0");
371+
if (query.version !== undefined) { feed(`v${query.version}`); feed("\0"); }
371372
for (const f of [...query.filters].sort((a, b) => a.column.localeCompare(b.column) || a.op.localeCompare(b.op))) {
372-
feed(f.column); feed(f.op); feed(String(f.value));
373+
feed(f.column); feed("\0"); feed(f.op); feed("\0"); feed(String(f.value)); feed("\0");
373374
}
374-
for (const p of [...query.projections].sort()) feed(p);
375-
if (query.sortColumn) { feed(query.sortColumn); feed(query.sortDirection ?? "asc"); }
376-
if (query.limit !== undefined) feed(String(query.limit));
377-
if (query.offset !== undefined) feed(String(query.offset));
378375
if (query.filterGroups) {
379376
for (const group of query.filterGroups) {
380377
feed("|");
381378
for (const f of [...group].sort((a, b) => a.column.localeCompare(b.column) || a.op.localeCompare(b.op))) {
382-
feed(f.column); feed(f.op); feed(String(f.value));
379+
feed(f.column); feed("\0"); feed(f.op); feed("\0"); feed(String(f.value)); feed("\0");
383380
}
384381
}
385382
}
386-
if (query.aggregates) for (const a of query.aggregates) { feed(a.fn); feed(a.column); if (a.alias) feed(a.alias); }
387-
if (query.groupBy) for (const g of query.groupBy) feed(g);
388-
if (query.distinct) feed("distinct");
389-
if (query.version !== undefined) feed(`v${query.version}`);
390-
if (query.windows) for (const w of query.windows) { feed(w.fn); feed(w.alias); feed(w.column ?? ""); }
391-
if (query.computedColumns) for (const cc of query.computedColumns) feed(cc.name);
392-
if (query.setOperation) { feed(query.setOperation.type); feed(query.setOperation.table); }
393-
if (query.subqueryIn) { feed(query.subqueryIn.column); feed(query.subqueryIn.table); }
383+
for (const p of [...query.projections].sort()) { feed(p); feed("\0"); }
384+
if (query.sortColumn) { feed(query.sortColumn); feed("\0"); feed(query.sortDirection ?? "asc"); feed("\0"); }
385+
if (query.limit !== undefined) { feed(String(query.limit)); feed("\0"); }
386+
if (query.offset !== undefined) { feed(String(query.offset)); feed("\0"); }
387+
if (query.aggregates) for (const a of query.aggregates) { feed(a.fn); feed("\0"); feed(a.column); feed("\0"); if (a.alias) feed(a.alias); feed("\0"); }
388+
if (query.groupBy) for (const g of query.groupBy) { feed(g); feed("\0"); }
389+
if (query.distinct) for (const d of query.distinct) { feed(d); feed("\0"); }
390+
if (query.windows) for (const w of query.windows) {
391+
feed(w.fn); feed("\0"); feed(w.alias); feed("\0"); feed(w.column ?? ""); feed("\0");
392+
if (w.partitionBy) for (const p of w.partitionBy) { feed(p); feed("\0"); }
393+
if (w.orderBy) for (const o of w.orderBy) { feed(o.column); feed(o.direction); feed("\0"); }
394+
if (w.frame) { feed(w.frame.type); feed(String(w.frame.start)); feed(String(w.frame.end)); feed("\0"); }
395+
if (w.args?.offset !== undefined) { feed(String(w.args.offset)); feed("\0"); }
396+
}
397+
if (query.computedColumns) for (const cc of query.computedColumns) { feed(cc.alias); feed("\0"); }
398+
if (query.setOperation) { feed(query.setOperation.mode); feed("\0"); feed(this.queryCacheKey(query.setOperation.right)); feed("\0"); }
399+
if (query.subqueryIn) for (const sq of query.subqueryIn) { feed(sq.column); feed("\0"); for (const v of sq.valueSet) { feed(v); feed("\0"); } }
400+
if (query.join) { feed(query.join.type ?? "inner"); feed("\0"); feed(query.join.leftKey); feed("\0"); feed(query.join.rightKey); feed("\0"); feed(this.queryCacheKey(query.join.right)); feed("\0"); }
394401
return `qr:${query.table}:${(h >>> 0).toString(36)}`;
395402
}
396403

src/query-do.ts

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -662,29 +662,37 @@ export class QueryDO extends DurableObject<Env> {
662662
// FNV-1a hash over query components — no JSON serialization
663663
let h = 0x811c9dc5;
664664
const feed = (s: string) => { for (let i = 0; i < s.length; i++) { h ^= s.charCodeAt(i); h = Math.imul(h, 0x01000193); } };
665-
feed(query.table);
665+
feed(query.table); feed("\0");
666+
if (query.version !== undefined) { feed(`v${query.version}`); feed("\0"); }
666667
for (const f of [...query.filters].sort((a, b) => a.column.localeCompare(b.column) || a.op.localeCompare(b.op))) {
667-
feed(f.column); feed(f.op); feed(String(f.value));
668+
feed(f.column); feed("\0"); feed(f.op); feed("\0"); feed(String(f.value)); feed("\0");
668669
}
669670
if (query.filterGroups) {
670671
for (const group of query.filterGroups) {
671-
feed("|"); // group separator
672+
feed("|");
672673
for (const f of [...group].sort((a, b) => a.column.localeCompare(b.column) || a.op.localeCompare(b.op))) {
673-
feed(f.column); feed(f.op); feed(String(f.value));
674+
feed(f.column); feed("\0"); feed(f.op); feed("\0"); feed(String(f.value)); feed("\0");
674675
}
675676
}
676677
}
677-
for (const p of [...query.projections].sort()) feed(p);
678-
if (query.sortColumn) { feed(query.sortColumn); feed(query.sortDirection ?? "asc"); }
679-
if (query.limit !== undefined) feed(String(query.limit));
680-
if (query.offset !== undefined) feed(String(query.offset));
681-
if (query.aggregates) for (const a of query.aggregates) { feed(a.fn); feed(a.column); if (a.alias) feed(a.alias); }
682-
if (query.groupBy) for (const g of query.groupBy) feed(g);
683-
if (query.distinct) feed("distinct");
684-
if (query.windows) for (const w of query.windows) { feed(w.fn); feed(w.alias); feed(w.column ?? ""); }
685-
if (query.computedColumns) for (const cc of query.computedColumns) feed(cc.name);
686-
if (query.setOperation) { feed(query.setOperation.type); feed(query.setOperation.table); }
687-
if (query.subqueryIn) { feed(query.subqueryIn.column); feed(query.subqueryIn.table); }
678+
for (const p of [...query.projections].sort()) { feed(p); feed("\0"); }
679+
if (query.sortColumn) { feed(query.sortColumn); feed("\0"); feed(query.sortDirection ?? "asc"); feed("\0"); }
680+
if (query.limit !== undefined) { feed(String(query.limit)); feed("\0"); }
681+
if (query.offset !== undefined) { feed(String(query.offset)); feed("\0"); }
682+
if (query.aggregates) for (const a of query.aggregates) { feed(a.fn); feed("\0"); feed(a.column); feed("\0"); if (a.alias) feed(a.alias); feed("\0"); }
683+
if (query.groupBy) for (const g of query.groupBy) { feed(g); feed("\0"); }
684+
if (query.distinct) for (const d of query.distinct) { feed(d); feed("\0"); }
685+
if (query.windows) for (const w of query.windows) {
686+
feed(w.fn); feed("\0"); feed(w.alias); feed("\0"); feed(w.column ?? ""); feed("\0");
687+
if (w.partitionBy) for (const p of w.partitionBy) { feed(p); feed("\0"); }
688+
if (w.orderBy) for (const o of w.orderBy) { feed(o.column); feed(o.direction); feed("\0"); }
689+
if (w.frame) { feed(w.frame.type); feed(String(w.frame.start)); feed(String(w.frame.end)); feed("\0"); }
690+
if (w.args?.offset !== undefined) { feed(String(w.args.offset)); feed("\0"); }
691+
}
692+
if (query.computedColumns) for (const cc of query.computedColumns) { feed(cc.alias); feed("\0"); }
693+
if (query.setOperation) { feed(query.setOperation.mode); feed("\0"); feed(this.queryKey(query.setOperation.right)); feed("\0"); }
694+
if (query.subqueryIn) for (const sq of query.subqueryIn) { feed(sq.column); feed("\0"); for (const v of sq.valueSet) { feed(v); feed("\0"); } }
695+
if (query.join) { feed(query.join.type ?? "inner"); feed("\0"); feed(query.join.leftKey); feed("\0"); feed(query.join.rightKey); feed("\0"); feed(this.queryKey(query.join.right)); feed("\0"); }
688696
return `qr:${query.table}:${(h >>> 0).toString(36)}`;
689697
}
690698

src/query-schema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,8 @@ const vectorSearchSchema = z.object({
3434
});
3535

3636
export const queryDescriptorSchema = z.object({
37-
table: z.string().min(1, "Table name is required"),
37+
table: z.string().min(1, "Table name is required")
38+
.refine(s => !s.includes("..") && !s.startsWith("/"), "Invalid table name"),
3839
filters: z.array(filterOpSchema).default([]),
3940
projections: z.array(z.string()).default([]),
4041
select: z.array(z.string()).optional(), // alias for projections

src/types.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,8 @@ export interface Env {
356356
MASTER_DO: DurableObjectNamespace;
357357
QUERY_DO: DurableObjectNamespace;
358358
FRAGMENT_DO: DurableObjectNamespace;
359+
/** Set to truthy in wrangler.toml [vars] to enable /upload endpoint. */
360+
DEV_MODE?: string;
359361
}
360362

361363
/** RPC interface exposed by QueryDO for zero-serialization calls */

src/worker.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,10 +78,13 @@ export default {
7878
return json(base, 200, headers);
7979
}
8080

81-
// Direct R2 upload (local dev only)
81+
// Direct R2 upload (local dev only — blocked in production)
8282
if (url.pathname === "/upload" && request.method === "POST") {
83+
if (!env.DEV_MODE) return json({ error: "upload disabled" }, 403);
8384
const key = url.searchParams.get("key");
84-
if (!key) return new Response("Missing ?key=", { status: 400 });
85+
if (!key || key.includes("..") || key.startsWith("/")) {
86+
return json({ error: "invalid key" }, 400);
87+
}
8588
await resolveBucket(env, key).put(key, request.body);
8689
return json({ uploaded: key });
8790
}

0 commit comments

Comments
 (0)