Page 1 of 1

Security Considerations

Posted: Sun Jan 31, 2021 5:30 am
by apmuthu
The Wiki article Security Considerations is not only specific to nuBuilder but has sever general aspects in choosing and deploying Open Source Applications as well. It has been curated from the dev discussions and credits go out to the participants viz., @Kyle, @amit and the regular devs.

The attachment is a snapshot of the article as on date and can be useful to those in need of justifying the use of any Open Source application and specifically nuBuilder to both themselves and those in the hierarchy of decision making.
nuBuilder Security Considerations.pdf

Re: Security Considerations

Posted: Tue Jan 04, 2022 3:31 am
by joellimardo
I'm perusing the code and I keep seeing uses of POST data embedded in SQL and then passed in as-is to nuRunQuery* methods in nudatabase.php where simple prepare() and execute() are being used without any sanitization that I can see. According to online information regarding PDO this style of coding does not adequately protect against SQL injection attacks. Apparently the expectation is that the code will use placeholders.

I am fairly new to the codebase so if someone can point me to how the code does this sanitization step I would appreciate it. Thanks in advance.

Re: Security Considerations

Posted: Thu Jan 06, 2022 5:22 pm
by kev1n
A software like SonarQube could give us some insights.

SonarQube is an open-source platform developed by SonarSource for continuous inspection of code quality to perform automatic reviews with static analysis of code to detect bugs, code smells, and security vulnerabilities on 20+ programming languages. SonarQube offers reports on duplicated code, coding standards, unit tests, code coverage, code complexity, comments, bugs, and security vulnerabilities.

Re: Security Considerations

Posted: Fri Jan 07, 2022 10:51 am
by kev1n
3 (minor) potential vulnerabilities have found by https://detectify.com. Fixes are on Github.

Details here: viewtopic.php?t=11432

Re: Security Considerations

Posted: Fri Jan 07, 2022 11:14 am
by lxnunes
Hi Kevin, many thanks for the update.

If I may, I would like to raise a question about the existing method of hashing the user passwords. nuBuilder is using MD5 and I would like to ask about the possibility to use instead the native password_hash(), mostly because of the low security of MD5.

I am not sure how much of a breaking change this would be, especially considering some of us use nuBuilder in Arduinos... You can see a discussion on the topic in https://stackoverflow.com/questions/189 ... sword-hash

best

Re: Security Considerations

Posted: Fri Jan 07, 2022 11:37 am
by kev1n
Hi,

Thanks for bringing this to our attention.

I think we can proceed as described in a stackoverflow post:

When the user tries to log in take the following steps to modify your login process accordingly.
  • Check if the hash in the db starts with $2y$ to determine if the password should be check with md5 or password_verify. If it does start with $2y$ then just use password_verify and ignore the remaining steps (continuing on with the rest of your normal login process).
  • If the password hash in the database does not start with $2y$ then first, check the plain-text password against its md5 hash.
  • If the plain-text password's hash doesn't matches the md5 hash in your database continue with normal failed authentication process and ignore the remaining steps here
  • If the plain-text password's hash does match the md5 hash in your database then take the plain-text password and run it through password_hash and update your database with the newly generated BCRYPT hash from password_hash.
You would have to keep this code in your login process until all passwords in your database have been updated and no remaining md5 hashes are left. The user will never know that their password hash is updated and never be prompted to enter their password twice as it's completely unnecessary.

Re: Security Considerations

Posted: Fri Jan 07, 2022 12:29 pm
by lxnunes
In my case, the user database will be created fresh, so all my passwords will use the new format.

However, in case I had plenty of MD5 passwords (or users that do not connetc very often), I am not sure exactly how I would proceed, especially because keeping the code for e.g. more than a year, would not make much sense. If user hasn't logged-in in that time, I might as well update their password directly to whatever and let him/her reset it later if needed.

Re: Security Considerations

Posted: Fri Jan 07, 2022 8:30 pm
by kev1n
If you guys would like to test the new password_hash() function, download the core.zip from the attachment, unzip the file and replace the 3 files under the /core directory (create a backup first)

How it works:

- Log in with a user account (not globeadmin) and password
- If the password is correct, nuBuilder will replace the md5 password hash in the database with a new hash that is generated with password_hash()
- You should still be able to log in after the replacement

E.g. if your password is test its hash is 098f6bcd4621d373cade4e832627b4f6, which will be replaced with $2y$10$ktxW/Nm8rSWTXFzQfpj9huW.zv1fqNOzFsdg6MC8DR6vU4wSYc0Be

Re: Security Considerations

Posted: Sat Jan 08, 2022 2:24 pm
by lxnunes
Hi! I confirm it worked as described. Transparently changed the password of a normal user from MD5.
Cheers

Re: Security Considerations

Posted: Sat Jan 08, 2022 7:44 pm
by kev1n
Thanks for testing & confirming!