Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notifier server should only bind to loopback interface #93

Open
pixtron opened this issue Nov 11, 2018 · 2 comments
Open

Notifier server should only bind to loopback interface #93

pixtron opened this issue Nov 11, 2018 · 2 comments
Assignees

Comments

@pixtron
Copy link

pixtron commented Nov 11, 2018

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

server.listen(this.httpServerPort);

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
Copy link
Author

pixtron commented Nov 11, 2018

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').

@tikurahul tikurahul self-assigned this Nov 14, 2018
@tikurahul
Copy link
Collaborator

You are correct. This should be fixed.

pixtron added a commit to pixtron/AppAuth-JS that referenced this issue Dec 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants