[breaking] rename xForwardedForIndex to xForwardedForNumProxies

sveltejs · Mar 15, 2022 · 8e1e1ae · 8e1e1ae
1 parent f196861
commit 8e1e1ae
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 16 deletions.
diff --git a/packages/adapter-node/README.md b/packages/adapter-node/README.md
@@ -94,23 +94,17 @@ MY_ORIGINURL=https://my.site \
 node build
 ```
 
-### xForwardedForIndex
+### xForwardedForNumProxies
 
-If the `ADDRESS_HEADER` is `X-Forwarded-For`, the header value will contain a comma-separated list of IP addresses. For example, if there are three proxies between your server and the client, proxy 3 will forward the addresses of the client and the first two proxies:
+If the `ADDRESS_HEADER` is `X-Forwarded-For`, the header value will contain a comma-separated list of IP addresses. `xForwardedForNumProxies` should specify how many trusted proxies sit between your server and the client. E.g. if there are three trusted proxies, proxy 3 will forward the addresses of the client and the first two proxies:
 
 ```
 <client address>, <proxy 1 address>, <proxy 2 address>
 ```
 
-To get the client address we could use `xForwardedFor: 0` or `xForwardedFor: -3`, which counts back from the number of addresses.
+To get the client address we could use `xForwardedFor: 3`.
 
-**X-Forwarded-For is [trivial to spoof](https://adam-p.ca/blog/2022/03/x-forwarded-for/), howevever**:
-
-```
-<spoofed address>, <client address>, <proxy 1 address>, <proxy 2 address>
-```
-
-For that reason you should always use a negative number (depending on the number of proxies) if you need to trust `event.clientAddress`. In the above example, `0` would yield the spoofed address while `-3` would continue to work.
+If there may be a variable number of proxies, you would have to read the `X-Forwarded-For` header yourself and read the IP address from the left, but be very careful that you do not use the result for any applications with possible security implications.
 
 ## Custom server
 

diff --git a/packages/adapter-node/index.d.ts b/packages/adapter-node/index.d.ts
@@ -20,7 +20,7 @@ interface AdapterOptions {
 			host?: string;
 		};
 	};
-	xForwardedForIndex?: number;
+	xForwardedForNumProxies?: number;
 }
 
 declare function plugin(options?: AdapterOptions): Adapter;

diff --git a/packages/adapter-node/index.js b/packages/adapter-node/index.js
@@ -24,12 +24,16 @@ export default function ({
 			host: host_header_env = 'HOST_HEADER'
 		} = {}
 	} = {},
-	xForwardedForIndex = -1
+	xForwardedForNumProxies = 1
 } = {}) {
 	return {
 		name: '@sveltejs/adapter-node',
 
 		async adapt(builder) {
+			if (xForwardedForNumProxies < 1) {
+				throw new Error('xForwardedForNumProxies cannot be less than 1');
+			}
+
 			builder.rimraf(out);
 
 			builder.log.minor('Copying assets');
@@ -56,7 +60,7 @@ export default function ({
 					PROTOCOL_HEADER: JSON.stringify(protocol_header_env),
 					HOST_HEADER: JSON.stringify(host_header_env),
 					ADDRESS_HEADER: JSON.stringify(address_header_env),
-					X_FORWARDED_FOR_INDEX: JSON.stringify(xForwardedForIndex)
+					X_FORWARDED_FOR_PROXIES: JSON.stringify(xForwardedForNumProxies)
 				}
 			});
 

diff --git a/packages/adapter-node/src/handler.d.ts b/packages/adapter-node/src/handler.d.ts
@@ -5,7 +5,7 @@ declare global {
 	const ADDRESS_HEADER: string;
 	const HOST_HEADER: string;
 	const PROTOCOL_HEADER: string;
-	const X_FORWARDED_FOR_INDEX: number;
+	const X_FORWARDED_FOR_PROXIES: number;
 }
 
 export const handler: Handle;
diff --git a/packages/adapter-node/src/handler.js b/packages/adapter-node/src/handler.js
@@ -7,7 +7,7 @@ import { getRequest, setResponse } from '@sveltejs/kit/node';
 import { Server } from 'SERVER';
 import { manifest } from 'MANIFEST';
 
-/* global ORIGIN, ADDRESS_HEADER, PROTOCOL_HEADER, HOST_HEADER, X_FORWARDED_FOR_INDEX */
+/* global ORIGIN, ADDRESS_HEADER, PROTOCOL_HEADER, HOST_HEADER, X_FORWARDED_FOR_PROXIES */
 
 const server = new Server(manifest);
 const origin = ORIGIN;
@@ -62,7 +62,12 @@ const ssr = async (req, res) => {
 
 					if (address_header === 'x-forwarded-for') {
 						const addresses = value.split(',');
-						return addresses[(addresses.length + X_FORWARDED_FOR_INDEX) % addresses.length].trim();
+						if (X_FORWARDED_FOR_PROXIES > addresses.length) {
+							throw new Error(
+								`Received xForwardedForNumProxies of ${X_FORWARDED_FOR_PROXIES}, but only found ${addresses.length} addresses`
+							);
+						}
+						return addresses[addresses.length - X_FORWARDED_FOR_PROXIES].trim();
 					}
 
 					return value;