AppAuth-JS icon indicating copy to clipboard operation
AppAuth-JS copied to clipboard

Notifier server should only bind to loopback interface

Open pixtron opened this issue 7 years ago • 2 comments

Expected Behavior

Describe expected behavior

The notifier server should only listen on the loopback interface (127.0.0.1)

Describe the problem

Actual Behavior

The notifier server binds to 0.0.0.0 or - if ipv6 is available - ::, hence every client in the same network can access the notifier server. Another client in the same network might shut down the notifier server or inject auth tokens with a malicious request.

Steps to reproduce the behavior

1.) Connect two clients to the same network 2.) Client A: Start the example electron app (googlesamples/appauth-js-electron-sample) 3.) Client A: Click "Sign in" -> Browser window opens consent screen 4.) Client B: open http://192.168.0.101:8000/code=xxx (assuming client A has IP: 192.168.0.101) 5.) Electron app on client A logs the below output to the developer console of the BrowserWindow

Checking to see if there is an authorization response to be delivered. logger.ts:23 
Authorization request complete  AuthorizationRequest {…} 
AuthorizationResponse {code: "xxx", state: undefined} null logger.ts:21 
Request ended with an error  400 {error: "invalid_grant", error_description: "Malformed auth code."}
internal/process/next_tick.js:188 
Uncaught (in promise) AppAuthError {message: "Bad Request", extras: undefined}

Environment

  • AppAuth-JS version: Tested on 0.3.5 and 1.1.1
  • AppAuth-JS Environment: node

Possible Solutions

https://github.com/openid/AppAuth-JS/blob/6a9c47c9002829142f50ecc887b538216cef5854/src/node_support/node_request_handler.ts#L111

The above should be changed to:

server.listen(this.httpServerPort, '127.0.0.1');

Alternatively allow to configure the host(s):

this.httpServerHosts = [
  '127.0.0.1',
  '::1'
]

this.httpServerHosts.forEach(host => {
  server.listen(this.httpServerPort, host);
})

See: https://nodejs.org/docs/latest-v10.x/api/net.html#net_server_listen_port_host_backlog_callback

pixtron avatar Nov 11 '18 17:11 pixtron

Not sure if defaulting to '127.0.0.1' would break any apps relying on '0.0.0.0'. Therfore the best solution might be to allow configuration of the interface with a beautiful default (IMO this would be '127.0.0.1').

pixtron avatar Nov 11 '18 17:11 pixtron

You are correct. This should be fixed.

tikurahul avatar Nov 14 '18 03:11 tikurahul