Skip to content

Commit 3631854

Browse files
jasonsaaymanjasonsaayman
authored
fix: unrestricted cloud metadata exfiltration via header injection chain (#10660)
* fix: unrestricted cloud metadata exfiltration via header injection chain * fix: address pattern issue highlighted by cubic * fix: code ql feedback * fix: code ql feedback
1 parent fb3befb commit 3631854

6 files changed

Lines changed: 120 additions & 16 deletions

File tree

‎lib/core/Axios.js‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,29 @@ class Axios {
4646
Error.captureStackTrace ? Error.captureStackTrace(dummy) : (dummy = new Error());
4747

4848
// slice off the Error: ... line
49-
const stack = dummy.stack ? dummy.stack.replace(/^.+\n/, '') : '';
49+
const stack = (() => {
50+
if (!dummy.stack) {
51+
return '';
52+
}
53+
54+
const firstNewlineIndex = dummy.stack.indexOf('\n');
55+
56+
return firstNewlineIndex === -1 ? '' : dummy.stack.slice(firstNewlineIndex + 1);
57+
})();
5058
try {
5159
if (!err.stack) {
5260
err.stack = stack;
5361
// match without the 2 top stack lines
54-
} else if (stack && !String(err.stack).endsWith(stack.replace(/^.+\n.+\n/, ''))) {
55-
err.stack += '\n' + stack;
62+
} else if (stack) {
63+
const firstNewlineIndex = stack.indexOf('\n');
64+
const secondNewlineIndex =
65+
firstNewlineIndex === -1 ? -1 : stack.indexOf('\n', firstNewlineIndex + 1);
66+
const stackWithoutTwoTopLines =
67+
secondNewlineIndex === -1 ? '' : stack.slice(secondNewlineIndex + 1);
68+
69+
if (!String(err.stack).endsWith(stackWithoutTwoTopLines)) {
70+
err.stack += '\n' + stack;
71+
}
5672
}
5773
} catch (e) {
5874
// ignore the case where "stack" is an un-writable property

‎lib/core/AxiosHeaders.js‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,49 @@ import parseHeaders from '../helpers/parseHeaders.js';
55

66
const $internals = Symbol('internals');
77

8+
const isValidHeaderValue = (value) => !/[\r\n]/.test(value);
9+
10+
function assertValidHeaderValue(value, header) {
11+
if (value === false || value == null) {
12+
return;
13+
}
14+
15+
if (utils.isArray(value)) {
16+
value.forEach((v) => assertValidHeaderValue(v, header));
17+
return;
18+
}
19+
20+
if (!isValidHeaderValue(String(value))) {
21+
throw new Error(`Invalid character in header content ["${header}"]`);
22+
}
23+
}
24+
825
function normalizeHeader(header) {
926
return header && String(header).trim().toLowerCase();
1027
}
1128

29+
function stripTrailingCRLF(str) {
30+
let end = str.length;
31+
32+
while (end > 0) {
33+
const charCode = str.charCodeAt(end - 1);
34+
35+
if (charCode !== 10 && charCode !== 13) {
36+
break;
37+
}
38+
39+
end -= 1;
40+
}
41+
42+
return end === str.length ? str : str.slice(0, end);
43+
}
44+
1245
function normalizeValue(value) {
1346
if (value === false || value == null) {
1447
return value;
1548
}
1649

17-
return utils.isArray(value)
18-
? value.map(normalizeValue)
19-
: String(value).replace(/[\r\n]+$/, '');
50+
return utils.isArray(value) ? value.map(normalizeValue) : stripTrailingCRLF(String(value));
2051
}
2152

2253
function parseTokens(str) {
@@ -98,6 +129,7 @@ class AxiosHeaders {
98129
_rewrite === true ||
99130
(_rewrite === undefined && self[key] !== false)
100131
) {
132+
assertValidHeaderValue(_value, _header);
101133
self[key || _header] = normalizeValue(_value);
102134
}
103135
}

‎tests/browser/adapter.browser.test.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ class MockXMLHttpRequest {
1414
this.upload = {
1515
addEventListener() {},
1616
};
17+
this.requestHeaders = {};
1718
}
1819

1920
open(method, url, async = true) {
@@ -22,7 +23,9 @@ class MockXMLHttpRequest {
2223
this.async = async;
2324
}
2425

25-
setRequestHeader() {}
26+
setRequestHeader(key, value) {
27+
this.requestHeaders[key] = value;
28+
}
2629

2730
addEventListener() {}
2831

@@ -175,4 +178,16 @@ describe('adapter (vitest browser)', () => {
175178
request.respondWith();
176179
await responsePromise;
177180
});
181+
182+
it('should reject request headers containing CRLF characters', async () => {
183+
await expect(
184+
axios('/foo', {
185+
headers: {
186+
'x-test': 'ok\r\nInjected: yes',
187+
},
188+
})
189+
).rejects.toThrow(/Invalid character in header content/);
190+
191+
expect(requests.length).toBe(0);
192+
});
178193
});

‎tests/unit/adapters/fetch.test.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,17 @@ const fetchAxios = axios.create({
2626
});
2727

2828
describe.runIf(typeof fetch === 'function')('supports fetch with nodejs', () => {
29+
it('should reject request headers containing CRLF characters', async () => {
30+
await assert.rejects(
31+
fetchAxios.get(`${LOCAL_SERVER_URL}/`, {
32+
headers: {
33+
'x-test': 'ok\r\nInjected: yes',
34+
},
35+
}),
36+
/(invalid.*header|header.*invalid)/i
37+
);
38+
});
39+
2940
describe('responses', () => {
3041
it('should support text response type', async () => {
3142
const originalData = 'my data';

‎tests/unit/adapters/http.test.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,17 @@ describe('supports http with nodejs', () => {
125125
}
126126
});
127127

128+
it('should reject request headers containing CRLF characters', async () => {
129+
await assert.rejects(
130+
axios.get('http://localhost:1/', {
131+
headers: {
132+
'x-test': 'ok\r\nInjected: yes',
133+
},
134+
}),
135+
/Invalid character in header content/
136+
);
137+
});
138+
128139
it('should parse the timeout property', async () => {
129140
const server = await startHTTPServer(
130141
(req, res) => {

‎tests/unit/axiosHeaders.test.js‎

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,19 +85,38 @@ describe('AxiosHeaders', () => {
8585
});
8686

8787
const runIfNode18OrHigher = nodeMajorVersion >= 18 ? it : it.skip;
88-
runIfNode18OrHigher('should support setting multiple header values from an iterable source', () => {
88+
runIfNode18OrHigher(
89+
'should support setting multiple header values from an iterable source',
90+
() => {
91+
const headers = new AxiosHeaders();
92+
const nativeHeaders = new Headers();
93+
94+
nativeHeaders.append('set-cookie', 'foo');
95+
nativeHeaders.append('set-cookie', 'bar');
96+
nativeHeaders.append('set-cookie', 'baz');
97+
nativeHeaders.append('y', 'qux');
98+
99+
headers.set(nativeHeaders);
100+
101+
assert.deepStrictEqual(headers.get('set-cookie'), ['foo', 'bar', 'baz']);
102+
assert.strictEqual(headers.get('y'), 'qux');
103+
}
104+
);
105+
106+
it('should throw on CRLF in header value', () => {
89107
const headers = new AxiosHeaders();
90-
const nativeHeaders = new Headers();
91108

92-
nativeHeaders.append('set-cookie', 'foo');
93-
nativeHeaders.append('set-cookie', 'bar');
94-
nativeHeaders.append('set-cookie', 'baz');
95-
nativeHeaders.append('y', 'qux');
109+
assert.throws(() => {
110+
headers.set('x-test', 'safe\r\nInjected: true');
111+
}, /Invalid character in header content/);
112+
});
96113

97-
headers.set(nativeHeaders);
114+
it('should throw on CRLF in any array header value', () => {
115+
const headers = new AxiosHeaders();
98116

99-
assert.deepStrictEqual(headers.get('set-cookie'), ['foo', 'bar', 'baz']);
100-
assert.strictEqual(headers.get('y'), 'qux');
117+
assert.throws(() => {
118+
headers.set('set-cookie', ['safe=1', 'unsafe=1\nInjected: true']);
119+
}, /Invalid character in header content/);
101120
});
102121
});
103122

0 commit comments

Comments
 (0)