Security Issue - WiFi Password

[melvinisken]

I now have one ESP32-Cam mounted and running since about 10 days. In general, it works quite well but I came across a severe security issue (I think). Last week the camera somehow lost the connection to the router and didn't seem to be able to reconnect after power cycling. I didn't have the chance to dig deeper the last days and left it as it was. Today I I checked a little closer and I noticed that it went into soft-ap mode. Using the cameras network, I did a soft-reboot and the camera reconnected to my network. So that works in the end, BUT: I (accidently) left the camera in this soft-ap mode for a couple of days. So everyone that came along my house could login and enter the camera configuration page which stored my wifi password in clear text (in the password field, hidden by dots, but you can reveal such a password with simple, free browser extensions). So I would suggest to leave the passwort field empty (do not load it to the webpage) and only update the passwort field when something has been filled in. When I have some additional time, I will try to create an example.

[bkeevil]

Is your development environment working? Mine is not.

Try commenting out line 379 in app_httpd.c

    p+=sprintf(p, "\"colorbar\":%u,", s->status.colorbar);
    p+=sprintf(p, "\"hostname\":\"%s\",", settings.hostname);
    p+=sprintf(p, "\"wifi_ssid\":\"%s\",", settings.wifi_ssid);
//    p+=sprintf(p, "\"wifi_password\":\"%s\",", settings.wifi_password);
    #ifdef CONFIG_MDNS_ENABLED
    p+=sprintf(p, "\"mdns_instance\":\"%s\",", settings.mdns_instance);
    #endif

This alone should be sufficient as the wifi password is sent from the web page with the password field onChange handler. If it is not changed then it will not be updated when the user clicks Save.

[bkeevil]

If we're going to do a proper change password feature, there are some hash functions in the esp-idf wpa_supplicant library here:

https://github.com/espressif/esp-idf/tree/master/components/wpa_supplicant/src/crypto

I think they may be linked in already so no a minimal increase in compiled code size.

[melvinisken]

I also found that line, removed it and it works in general. The only thing is, that the password field still gets filled, but with the string "undefined". From a user perspective, I would prefer having it empty, but the security issue itself is fixed, I think.

[bkeevil]

@karlitos I just applied this one line change to master but am unable to verify that it works. Would you mind doing a review before I close this?

[karlitos]

Yes, I am on it, regarding the "undefined" value it shall be easily fixable in JS

[karlitos]

It works and fine I made a PR for the 'undefined' value issue

[bkeevil]

Thanks @karlitos. Merged