medic / cht-user-management

GNU Affero General Public License v3.0
3 stars 1 forks source link

Cleartext passwords leaked via GET #42

Closed mrjones-plip closed 5 months ago

mrjones-plip commented 5 months ago

I just noticed that when logging in, the app does a GET request with the domain (CHT URL), username and password passed as a query string. This means that a number of systems just logged the password in the clear, by default:

Demo of this is shown in video below and here's a sample node app log entry:

[17:19:23.402] INFO (2624350): incoming request
    reqId: "req-3"
    req: {
      "method": "GET",
      "url": "/login?domain=192-168-68-26.local-ip.medicmobile.org%3A10463&username=username-here&password=password-here",
      "hostname": "127.0.0.2:3001",
      "remoteAddress": "127.0.0.1",
      "remotePort": 54870
    }

The fix this is to not use the query string. Instead use POST when transmitting login credentials. Alternately, HTTP Headers can be used, outside the per view of the query string. For more info, see OWASP's "Sensitive information in HTTP requests".

https://github.com/medic/cht-user-management/assets/8253488/f4320926-77cf-47b1-abf5-6e0e7093e04a

kennsippell commented 5 months ago

@mrjones-plip What browser are you using? I'm seeing POST requests and no-repro on both Chrome or Firefox. Do you reproduce this there?

mrjones-plip commented 5 months ago

oh no! So sorry @kennsippell. I'm using latest FF on Ubuntu 22. I had trouble reproducing it as well, and then I blocked unpkg.com with the plugin I use and the behavior manifested.

It's reasonable to assume no one is running the same prohibitive plugin I am - so closing ticket.

That said, I do think it would be nice if there weren't any third parties we were dependent on. Filed a ticket!