Security patch

Thank you for your contribution.
It will be studied for next release (possibly not for next corrective patch as some analysis and testing is required first).

Please notice that some systematic htmlEncode for ids (as you propose) may be a good idea (systematic security may always be a good idea) but is not a security leak when the id comes from database as it is always an integer (long number).
ProjeQtOr has been audited by independant security enterprises and pointed out few leaks that has instantly been fixed (before leaks have been published).
I played the OWASP ZAP tool on ProjeQtOr which did not find any leak on code (but found Joomla on the web root and found many issues on Joomla)
So I guess that most of the fix you propose are not really leaks.
ProjeQtOr has already a good production security level.
But your proposale is a good idea : all systematic encoding that will help ensure security are good ideas.

Thank you.

Hi,

Thanks for your long answer.
I will complete some points, not to build a sterile discussion, but to potentially reassure users who would read this topic and could be frightened.

The reason for the htmlEncode() is due to the fact that I couldn't trust data residing in the database due to various SQL injection flaws I noticed, so as a general precaution I made the general assumption that everything fetched from the database is potentially attacker controlled.

I share your point of view : it is a very good practice, and I will include your proposals in next version (after validation of course).
But you may consider that :
1) many of your patch concern htmlEncode(Object->id) or htmlEncode(Object->idOtherObject): as the field is in INT(12) format, there is absolutely no possibility that a hacker would have stored something that could genereate XSS (Cross Site Scripting).
2) "SQL injection flaws" may seem apparent on some files (some saveSomething.php files), but if you examine the persistence framework (/model/persistence/SqlElement.php) you'll seem that SQL injection is protected at this level. So ProjeQtOr is protected against SQL injection.
3) Framework and controls will not set possible to store or change any data in the database if the user is not connected. So before injecting data, a hacker must hack an account, or the hacker should be a member of your organisation.

There are several design flaws I noticed. Specifically there is no enforcement that data sent to the server has to come with POST or PUT methods and data retrieved from the server only using GET method.

Forcing POST or PUT is not very efficient in security : any hacker can send variable in PIT format, even if it is even more easy with a GET request.

There is no token required to be sent on POST requests (where the token is set by the server and sent to the client as a result of the GET request that displayed the HTML initiating the POST request. This enables CSRF attacks.

I agree it is a major way to tighten security. I'll keep this in mind for a next release (did not see this in your patch, but it is some heavy work to manage on all request...).

There's no proper filtering on data that is stored in session variables, allowing an attacker to replace any session variable with attacker controlled value (and as mentioned above, can be done using CSRF attack - imagine <img src="attack_url"></img> on a malicious site, where attack_url sets session variables to attacker controlled values).

This is true for variables used in parametring (date format, locale, ...). These data are not returned to the web page, but they could possibly set the mess in the interface. What is important to notice is that every security variable stored in session (user credentials, access rights) is stored in an object, so it is not possible for a hacker to store it directly. Only possibility is to erase or overwrite the object with a text variable, that will "disconnect" the session.

There's no proper validation against directory traversal attacks. An attacker can fetch any file they want from the server - including database passwords, etc...

Yes there is. It was a leak existing in V3.4.0, fixed in V3.4.1. If you really find a way to do this, please post it to me in private message at Cette adresse e-mail est protégée contre les robots spammeurs. Vous devez activer le JavaScript pour la visualiser..

Another example - you authenticate a user using md5 hash of username + microsecond, without taking into account that neither the username nor time are secrets.

I agree it is not completely secured, it is just a way to send data "not in clear text". The only way to really secure login transaction is to use https connection...

I have done as best as I can, within my own time constraints

And I really thank you for that.

I did not redesign the product and in some cases a redesign/rewrite would be the proper thing to do

But redesign is not planned. ProjeQtOr is not a banking system nor NSA website. Security is important but implementation must face risks.

Without (at least) the supplied fixes, running the code in a production environment exposes private information to attackers

I don't agree with you on that point. Even if code is not protected at level that will be required by NSA , security level seems correct for a Project Management Application, that will in most cases be installed on an intranet network, with credentials provided to a restricted population of users.

I will include your patch, but not in a hurry. I will not consider "urgency". I want to validate every change to be sure that the application will work correctly after patch is applied. This is the most important security constraint IMO, to avoid to have a hacker post some patch to introduce real leaks in the code.
I also want to be sure not to block some important feature "in the name of Security".
For instance, you disabled API (die; on fist line) : you may consider API is a security leak, but many users need it to interface ProjeQtOr with external applications. So my first change will be to define some parameter to enable or not API
But of course, if you can point out a proven security leak with an example, I will manage it as urgent (in that case, please sent it directly at Cette adresse e-mail est protégée contre les robots spammeurs. Vous devez activer le JavaScript pour la visualiser.).

Many thanks to your contribution, patch and remarks.
I really appreciate you involvement in making ProjeQtOr an even better application.