BlackCatDevelopment / BlackCatCMS

BlackCat CMS is a PHP5, HTML5 content management system
https://blackcat-cms.org
Other
11 stars 9 forks source link

general security #372

Closed DanielRuf closed 7 years ago

DanielRuf commented 7 years ago

Same for you like for all other forks / sources.

MD5, *rand and other solutions are insecure.

There are no prepared statements in many places, no properly sanitized inputs (XSS, RCE, SQLi, ...), almost no typecasting in many files and so on.

webbird commented 7 years ago

MD5 will be replaced with BC 2.0.

Please tell us where you found vulnerable statements or unsanitized input more precisely. "Many places", "many files", and so on, are not helpful.

Please note that we are not responsible for addons (Topics, Bakery, and so on).

webbird commented 7 years ago

See statement (German) here: https://forum.blackcat-cms.org/viewtopic.php?f=22&t=693

DanielRuf commented 7 years ago

See statement (German) here: https://forum.blackcat-cms.org/viewtopic.php?f=22&t=693

Unfortunately I neither have an account there nor have I more time currently to check all files and give you the exact lines where things are not ok.

Just as a sidenote, sanitizing passwords is not a good idea: https://github.com/BlackCatDevelopment/BlackCatCMS/blob/release-1.2/upload/framework/CAT/Users.php#L121

str* calls should be replaced with their mb_str* versions: https://github.com/BlackCatDevelopment/BlackCatCMS/blob/release-1.2/upload/framework/CAT/Users.php#L137

rand is not a CS(P)RNG https://github.com/BlackCatDevelopment/BlackCatCMS/blob/release-1.2/upload/framework/CAT/Users.php#L1343, use random_compat and random_bytes/int.

And so on and on. The files are huge.

That is definitely not enough: https://github.com/BlackCatDevelopment/BlackCatCMS/blob/release-1.2/upload/framework/class.secure.php#L265 There is much more than just simple script tags to bypass this ;-)

Unfortunately I have to get back to the other things now but I guess you get some idea that the overall security can be dramatically improved (and I bet this could have been done better with Symfony or Laravel and I find it a bit hard to read the code of such big files without a clear MVC strcuture and mixing view / content with classes which are not part of the view in the general way).

Don't get me wrong, this is a cool and huge project which much (handwritten?) code but not very clean, robust, extendable or modern imho and many mistakes are done because things are reinvented.

webbird commented 7 years ago

Thank you for your response. All the things you mentioned are focused with BlackCat CMS v2, which is a complete rewrite. You're right that we still have some issues inherited from WB/LEPTON, especially the users and rights management in version 1.x. (It was a basic decision to leave it as-is as it needs completely new concepts. The estimated timeline for BC2 development is 2 years!)

Of course we have thought about using a framework like Symfony, but we decided to do it our way. Using a framework does not make a project more "modern" by automatism. As this is kind of a matter of conviction, I am not going to discuss this here. If you're interested, you're invited to register in our forum (it's free, of course) or contact us by email.

webbird commented 7 years ago

Just as a sidenote, sanitizing passwords is not a good idea: https://github.com/BlackCatDevelopment/BlackCatCMS/blob/release-1.2/upload/framework/CAT/Users.php#L121

We do not sanitize the password there. As a spam protection method, the <input> field id/name for username and password is generated on every reload, so we need the name of that field given in another field. This is what is validated there. It's a relatively common method.

DanielRuf commented 7 years ago

My bad, sorry. Did not see the direct connection between the line and the field name.

webbird commented 7 years ago

No problem, you're not expected to know all the details. I don't remember them all, too. ;)

webbird commented 7 years ago

Reference: https://github.com/BlackCatDevelopment/BlackCatCMS_v2.0/issues/6

webbird commented 7 years ago

Moved to BC2.