Fix session and login vulnerabilities #2
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Session management
Currently the session secret is hard-coded to program code. This is a bad as knowing the session key allows the attacker overtake sessions bypassing the authorization.
Two steps are taken to mitigate this risk:
In
app.js
session secret is set to come from.env
file. If the session secret does not exist in that file, random 40 characters long hex string is generated instead.During the installation process random 40 character long hex string is set as session secret in
.env
file.Hard-coded credentials
At the moment login credentials are hard-coded in to the application code and although password is hashed it is never salted. This leaves it vulnerable for rainbow table style of attacks. Furthermore, as the source code for this project is public requiring username to match admin is essentially pointless, as anyone could just look up the username from the GitHub source.
These risks are also mitigated in two parts:
During the installation process user is prompted to enter an username and a password for the server, which will be stored in
.env
file.bcrypt
is used to salt and hash the user provided password.passport.js
will from now use username and password variables taken from the.env
file and usebcrypt
for hashing and salting instead ofmd5
.Impact on backwards compatibility
All the changes regarding to authentication are backwards compatible with current source code. Support for legacy
md5
hashed password is used, when.env
does not provide password variable.Session management change will however result in user being logged out after the server is restarted following the update. This logging out will occur on every server restart unless there exists a
.env
file, whereSESSION_SECRET
variable is specified.