View ProjeQtOr On SourceForge.net
ProjeQtOr - Project Management Tool
Support us on Capterra
OIN - Open Invention Network
ProjeQtOr free project management software - Security patch - ProjeQtOr
 
 

Security patch

More
25 Nov 2015 18:22 #1 by avris
Security patch was created by avris
I would like to state that Projeqtor is the best available open-source project management tool.
With that in mind, it is not ready for production deployment due to the large amount of security vulnerabilities present.
The attached patch, applied to a 5.1.3 distribution, should get it to a production safe state.
I would advise merging this, or a security equivalent (or better) patch, to your development branch, and apply these security checks for future code.
Attachments:

Please Log in or Create an account to join the conversation.

More
25 Nov 2015 18:42 #2 by babynus
Replied by babynus on topic 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.

Babynus
Administrator of ProjeQtOr web site
The following user(s) said Thank You: avris

Please Log in or Create an account to join the conversation.

More
25 Nov 2015 21:51 #3 by avris
Replied by avris on topic Security patch
You are welcome.
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.

All data that comes from external entities and is used as whole or in part as HTML should be properly sanitized to prevent cross-site-scripting attacks.

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. 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.

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).

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

There's no proper validation before attacker supplied data is used to instantiate objects - i.e.: some_var = new <attacker controlled string>(<attacker controlled string>); - potentially allowing remote code execution.

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 have done as best as I can, within my own time constraints, to get the system safe enough to be usable in a production environment. I did not redesign the product and in some cases a redesign/rewrite would be the proper thing to do. Without (at least) the supplied fixes, running the code in a production environment exposes private information to attackers - especially since now the fix is available, it will be studied by attackers and exploits (weather publiclly released or not) will be created to exploit these flaws - putting any user of the product at danger - so I don't think you should treat this lightly.

I agree that you need to study the code, but please keep the above in mind regarding urgency.

Thanks and Kind Regards,
Avri
The following user(s) said Thank You: babynus

Please Log in or Create an account to join the conversation.

More
26 Nov 2015 13:51 #4 by babynus
Replied by babynus on topic Security patch
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 This email address is being protected from spambots. You need JavaScript enabled to view it..

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 This email address is being protected from spambots. You need JavaScript enabled to view it.).

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

Babynus
Administrator of ProjeQtOr web site

Please Log in or Create an account to join the conversation.

Moderators: babynusprotion
Time to create page: 0.117 seconds

Cookies settings

×

Functional Cookies

Ce site utilise des cookies pour assurer son bon fonctionnement et ne peuvent pas être désactivés de nos systèmes. Nous ne les utilisons pas à des fins publicitaires. Si ces cookies sont bloqués, certaines parties du site ne pourront pas fonctionner.

Session

Please login to see yours activities!

Other cookies

Ce site web utilise un certain nombre de cookies pour gérer, par exemple, les sessions utilisateurs.