Arbitrary Client IP Address Injection?

[iamaldi]

Observations

I came across this function and am wondering why the application is relying on two specific HTTP request headers in order to find out the client's IP address.

https://github.com/apavlidi/IT_API/blob/880b8b7e8d8891d46eab9f5fcf43b4d51358d96d/routes/apiFunctions.js#L94-L96

As the user has complete control over the HTTP Requests, he/she can deliberately inject a custom IP address (probably a fake one) via the x-forwarded-for or x-real-ip in order to trick the back-end API to consume that as the client's IP address.

By observing the (boolean) checks performed by the getClientIp function, we can clearly see that the first checks are performed with the HTTP Request headers and then via the req.connection.remoteAddress which poses a risk as the user can achieve injection of a fake IP every time.

Consideration

Some testing needs to be done in order to check if my hypothesis on this holds true.

@apavlidi @kvisnia Please do let me know how exactly this function works and how its implementation is meant to work.

Thanks Aldi

[kvisnia]

Yes its probably a risk, the req.connection.remoteAddress should be first. The order which is set now (req.headers['x-forwarded-for'], req.headers['x-real-ip'], req.connection.remoteAddress), it's working "well" (lets say no one is messing with the headers) because the 'x-forwarded-for' is set by the proxy server we run in front of the nodejs application. We haven't tested the scenario you describing tho. Once we run the tests we can change the code accordingly.

[iamaldi]

@kvisnia I will perform that testing once my local setup is ready. From a security perspective I wouldn't suggest leaving the headers in the code. You shouldn't trust user data, so only the req.connection.remoteAddress is needed in this case as it will always return the user's IP address and not some user-supplied data as in the case of the req.headers['x-forwarded-for'], req.headers['x-real-ip'] headers.

[kvisnia]

For your tests to be accurate the setup has to be: Nodejs Application -> Web Proxy Server -> Client. The 'x-forwarded-for' header is altered by the proxy server, so if the user set lets say that header to '123.123.123.123', porxy server will add to it the client ip lets say '222.222.222.222' so the final header will have the value '123.123.123.123, 222.222.222.222'.

[iamaldi]

Why is that way though? Why do you allow the user to control a header for which the proxy needs to take into consideration?

---

From: Kostikas Visnia notifications@github.com Sent: Monday, December 3, 2018 4:22:49 PM To: apavlidi/IT_API Cc: Aldi; Author Subject: Re: [apavlidi/IT_API] Arbitrary Client IP Address Injection? (#57)

For you tests to be accurate the setup has to be: Nodejs Application -> Web Proxy Server -> Client. The 'x-forwarded-for' header is altered by the proxy server, so if the user set lets say that header to '123.123.123.123', porxy server will add to it the client ip lets say '222.222.222.222' so the final header will have the value '123.123.123.123, 222.222.222.222'.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHubhttps://github.com/apavlidi/IT_API/issues/57#issuecomment-443726688, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AVyUbhPdtDgP99pZAlh9KbG5oYNsfyvOks5u1TO5gaJpZM4Y9pvW.