OWASP / phpsec

OWASP PHP Security Project - THIS PROJECT IS INACTIVE AND MAY CONTAIN SECURITY FLAWS
197 stars 103 forks source link

Deleted code from repository and updated README to reflect abandoned status. #110

Closed AndrewCarterUK closed 8 years ago

AndrewCarterUK commented 8 years ago

Relates to: https://github.com/OWASP/phpsec/issues/108

abiusx commented 8 years ago

Please update the README file with the issues that exist in the code, not with a general explanation paragraph. And do not remove any of the code. (You can add readme files to separate folders if they are insecure as well).

AndrewCarterUK commented 8 years ago

Evidence of some of the vulnerabilities that warrant this action:

https://github.com/OWASP/phpsec/blob/master/libs/crypto/confidentialstring.php#L37

Hard coded encryption keys in a class that pretends to provide encryption (but actually just obfuscates). An object injection vulnerability was also found in this file.

https://github.com/OWASP/phpsec/blob/master/libs/core/random.php

        //If openssl is present, use that to generate random.
        if (function_exists("openssl_random_pseudo_bytes") && FALSE)
        {
            $random32bit=(int)(hexdec(bin2hex(openssl_random_pseudo_bytes(64))));
        }

The '&& FALSE' means that the CSPRNG openssl API is never used. Even if it was to be used the return value isn't checked (it can be false) and neither is the optional '$secure' output variable which flags whether the CSPRNG managed to securely generate a random value.

The intended fallback uses 'mt_rand' which should never be used for the purposes of cryptography. It also merges the seed values in a way that gives a ridiculously high number of collisions. Similar mistakes are made throughout the file.

https://github.com/OWASP/phpsec/blob/master/libs/auth/user.php#L71

This method converts the password to lower case before hashing it. This massively increases the opportunity for a brute force attack because 'PASSWORD' is the same as 'password'.

https://github.com/OWASP/phpsec/blob/master/libs/auth/user.php#L88

This method is vulnerable to timing attacks (should use hash_equals).

All of these were discovered in code inspections on only a few of the files that took about 15 minutes.

As a result it is quite clear that the code in this library diminishes the OWASP brand and could cause significant issues to anyone who uses it.

abiusx commented 8 years ago

Lets add that description to the bottom of the current README file, with a title like "UPDATE Nov2015" so that users can see and realize the issues with this library.

Thank you.

AndrewCarterUK commented 8 years ago

@abiusx This pull request is to delete the entire code base from the repository.

As I have already stated, it is beyond the point of repair and there is nobody willing to maintain it.

I would like you to bring this pull request to the attention of other OWASP team members if possible. With all due respect I do not believe we will make any further progress and it clearly needs the opinion of other members of your team.

abiusx commented 8 years ago

Unfortunately I cannot accept that. Removal of the entire code is not a beneficial action from what I see.

OWASP is an open source organization with thousands of members. There are no teams for this project. Any OWASP member or individual can contribute to OWASP.