Skip to content

Commit b185c0a

Browse files
hansotthansott
authored
Merge pull request #533 from AikidoSec/base-url-ssrf
Ignore base URL for outbound requests
2 parents 5db7d7f + 1426e73 commit b185c0a

File tree

10 files changed

+205
-263
lines changed

10 files changed

+205
-263
lines changed

‎end2end/tests/express-mongodb.ssrf.test.js‎

Lines changed: 81 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ t.test("it blocks in blocking mode", (t) => {
6666
});
6767

6868
server.on("error", (err) => {
69-
t.fail(err.message);
69+
t.fail(err);
7070
});
7171

7272
let stdout = "";
@@ -92,14 +92,35 @@ t.test("it blocks in blocking mode", (t) => {
9292
signal: AbortSignal.timeout(5000),
9393
}
9494
),
95+
fetch(
96+
`http://local.aikido.io:4000/images/${encodeURIComponent("http://local.aikido.io:4000")}`,
97+
{
98+
signal: AbortSignal.timeout(5000),
99+
headers: {
100+
Origin: "http://local.aikido.io:4000",
101+
Referer: "http://local.aikido.io:4000",
102+
},
103+
}
104+
),
105+
fetch(
106+
`http://local.aikido.io:4000/images/${encodeURIComponent("http://local.aikido.io:5875")}`,
107+
{
108+
signal: AbortSignal.timeout(5000),
109+
}
110+
),
95111
]);
96112
})
97-
.then(([safeRequest, ssrfRequest]) => {
113+
.then(([safeRequest, ssrfRequest, requestToItself, differentPort]) => {
98114
t.equal(safeRequest.status, 200);
99115
t.equal(ssrfRequest.status, 500);
100116
t.match(stdout, /Starting agent/);
101117
t.match(stderr, /Zen has blocked a server-side request forgery/);
102118

119+
// Requests to same hostname as the server should be allowed
120+
t.equal(requestToItself.status, 200);
121+
// If the port is different, it should be blocked
122+
t.equal(differentPort.status, 500);
123+
103124
return fetch(`${testServerUrl}/api/runtime/events`, {
104125
method: "GET",
105126
headers: {
@@ -114,14 +135,14 @@ t.test("it blocks in blocking mode", (t) => {
114135
const attacks = events.filter(
115136
(event) => event.type === "detected_attack"
116137
);
117-
t.same(attacks.length, 1);
138+
t.same(attacks.length, 2);
118139
const [attack] = attacks;
119140
t.match(attack.attack.stack, /app\.js/);
120141
t.match(attack.attack.stack, /fetchImage\.js/);
121142
t.match(attack.attack.stack, /express-async-handler/);
122143
})
123144
.catch((error) => {
124-
t.fail(error.message);
145+
t.fail(error);
125146
})
126147
.finally(() => {
127148
server.kill();
@@ -142,6 +163,10 @@ t.test("it does not block in dry mode", (t) => {
142163
t.end();
143164
});
144165

166+
server.on("error", (err) => {
167+
t.fail(err);
168+
});
169+
145170
let stdout = "";
146171
server.stdout.on("data", (data) => {
147172
stdout += data.toString();
@@ -174,7 +199,58 @@ t.test("it does not block in dry mode", (t) => {
174199
t.notMatch(stderr, /Zen has blocked a server-side request forgery/);
175200
})
176201
.catch((error) => {
177-
t.fail(error.message);
202+
t.fail(error);
203+
})
204+
.finally(() => {
205+
server.kill();
206+
});
207+
});
208+
209+
t.test("it blocks request to base URL if proxy is not trusted", (t) => {
210+
const server = spawn(`node`, [pathToApp, "4002"], {
211+
env: {
212+
...process.env,
213+
AIKIDO_DEBUG: "true",
214+
AIKIDO_BLOCKING: "true",
215+
AIKIDO_TOKEN: token,
216+
AIKIDO_URL: testServerUrl,
217+
AIKIDO_TRUST_PROXY: "false",
218+
},
219+
});
220+
221+
server.on("close", () => {
222+
t.end();
223+
});
224+
225+
server.on("error", (err) => {
226+
t.fail(err);
227+
});
228+
229+
let stdout = "";
230+
server.stdout.on("data", (data) => {
231+
stdout += data.toString();
232+
});
233+
234+
let stderr = "";
235+
server.stderr.on("data", (data) => {
236+
stderr += data.toString();
237+
});
238+
239+
// Wait for the server to start
240+
timeout(2000)
241+
.then(() => {
242+
return fetch(
243+
`http://local.aikido.io:4002/images/${encodeURIComponent("http://local.aikido.io:4002")}`,
244+
{
245+
signal: AbortSignal.timeout(5000),
246+
}
247+
);
248+
})
249+
.then((requestToItself) => {
250+
t.equal(requestToItself.status, 500);
251+
})
252+
.catch((error) => {
253+
t.fail(error);
178254
})
179255
.finally(() => {
180256
server.kill();

‎library/helpers/getIPAddressFromRequest.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { isIP } from "net";
22
import { isPrivateIP } from "../vulnerabilities/ssrf/isPrivateIP";
3+
import { trustProxy } from "./trustProxy";
34

45
export function getIPAddressFromRequest(req: {
56
headers: Record<string, unknown>;
@@ -57,16 +58,3 @@ function getClientIpFromXForwardedFor(value: string) {
5758

5859
return null;
5960
}
60-
61-
function trustProxy() {
62-
if (!process.env.AIKIDO_TRUST_PROXY) {
63-
// Trust proxy by default
64-
// Most of the time, the application is behind a reverse proxy
65-
return true;
66-
}
67-
68-
return (
69-
process.env.AIKIDO_TRUST_PROXY === "1" ||
70-
process.env.AIKIDO_TRUST_PROXY === "true"
71-
);
72-
}

‎library/helpers/trustProxy.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
import * as t from "tap";
2+
import { trustProxy } from "./trustProxy";
3+
4+
t.beforeEach(() => {
5+
delete process.env.AIKIDO_TRUST_PROXY;
6+
});
7+
8+
t.test("the default is true", async () => {
9+
t.equal(trustProxy(), true);
10+
});
11+
12+
t.test("trust proxy set to false", async () => {
13+
process.env.AIKIDO_TRUST_PROXY = "false";
14+
t.equal(trustProxy(), false);
15+
});
16+
17+
t.test("trust proxy set to true", async () => {
18+
process.env.AIKIDO_TRUST_PROXY = "true";
19+
t.equal(trustProxy(), true);
20+
});

‎library/helpers/trustProxy.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import { envToBool } from "./envToBool";
2+
3+
export function trustProxy() {
4+
if (!process.env.AIKIDO_TRUST_PROXY) {
5+
// Trust proxy by default
6+
// Most of the time, the application is behind a reverse proxy
7+
return true;
8+
}
9+
10+
return envToBool(process.env.AIKIDO_TRUST_PROXY);
11+
}

‎library/sinks/Fetch.test.ts‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -43,20 +43,22 @@ wrap(dns, "lookup", function lookup(original) {
4343
};
4444
});
4545

46-
const context: Context = {
47-
remoteAddress: "::1",
48-
method: "POST",
49-
url: "http://localhost:4000",
50-
query: {},
51-
headers: {},
52-
body: {
53-
image: "http://localhost:4000/api/internal",
54-
},
55-
cookies: {},
56-
routeParams: {},
57-
source: "express",
58-
route: "/posts/:id",
59-
};
46+
function createContext(): Context {
47+
return {
48+
remoteAddress: "::1",
49+
method: "POST",
50+
url: "http://local.aikido.io:4000",
51+
query: {},
52+
headers: {},
53+
body: {
54+
image: "http://localhost:4000/api/internal",
55+
},
56+
cookies: {},
57+
routeParams: {},
58+
source: "express",
59+
route: "/posts/:id",
60+
};
61+
}
6062

6163
const redirectTestUrl = "http://ssrf-redirects.testssandbox.com";
6264
const redirecTestUrl2 =
@@ -113,7 +115,7 @@ t.test(
113115

114116
agent.getHostnames().clear();
115117

116-
await runWithContext(context, async () => {
118+
await runWithContext(createContext(), async () => {
117119
// Don't await fetch to see how it handles
118120
// multiple requests at the same time
119121
// Because there's a single instance of the dispatcher
@@ -178,7 +180,7 @@ t.test(
178180

179181
await runWithContext(
180182
{
181-
...context,
183+
...createContext(),
182184
...{
183185
body: {
184186
image2: [
@@ -221,7 +223,7 @@ t.test(
221223

222224
await runWithContext(
223225
{
224-
...context,
226+
...createContext(),
225227
...{ body: { image: redirectUrl.ip } },
226228
},
227229
async () => {
@@ -238,7 +240,7 @@ t.test(
238240

239241
await runWithContext(
240242
{
241-
...context,
243+
...createContext(),
242244
...{ body: { image: redirectUrl.domain } },
243245
},
244246
async () => {
@@ -255,7 +257,7 @@ t.test(
255257

256258
await runWithContext(
257259
{
258-
...context,
260+
...createContext(),
259261
...{ body: { image: redirectUrl.ipTwice } },
260262
},
261263
async () => {
@@ -272,7 +274,7 @@ t.test(
272274

273275
await runWithContext(
274276
{
275-
...context,
277+
...createContext(),
276278
...{ body: { image: redirectUrl.domainTwice } },
277279
},
278280
async () => {
@@ -291,7 +293,7 @@ t.test(
291293

292294
await runWithContext(
293295
{
294-
...context,
296+
...createContext(),
295297
...{ body: { image: redirectUrl.ipv6 } },
296298
},
297299
async () => {
@@ -308,7 +310,7 @@ t.test(
308310

309311
await runWithContext(
310312
{
311-
...context,
313+
...createContext(),
312314
...{ body: { image: redirectUrl.ipv6Twice } },
313315
},
314316
async () => {
@@ -325,7 +327,7 @@ t.test(
325327

326328
await runWithContext(
327329
{
328-
...context,
330+
...createContext(),
329331
...{
330332
body: {
331333
image: `${redirecTestUrl2}/ssrf-test-absolute-domain`,
@@ -349,7 +351,7 @@ t.test(
349351
// Manual redirect
350352
await runWithContext(
351353
{
352-
...context,
354+
...createContext(),
353355
...{ body: { image: redirectUrl.ip } },
354356
},
355357
async () => {
@@ -373,7 +375,7 @@ t.test(
373375
// Manual redirect
374376
await runWithContext(
375377
{
376-
...context,
378+
...createContext(),
377379
...{ body: { image: redirectUrl.domain } },
378380
},
379381
async () => {
@@ -398,7 +400,7 @@ t.test(
398400

399401
await runWithContext(
400402
{
401-
...context,
403+
...createContext(),
402404
...{
403405
body: {
404406
image: `${redirecTestUrl2}/ssrf-test-absolute-domain`,

0 commit comments

Comments
 (0)