Security issues 2017 to review

[arnisjuraga]

opencart/opencart#5965 (comment)

[prhost]

cli_install removed from copona

[arnisjuraga]

Yes, I know, this was more like reminder for not recommended multiple 0777 https://github.com/copona/copona/search?utf8=%E2%9C%93&q=0777&type=

[ADDCreative]

Still a few security issues that haven't been addressed.

CSRF vulnerability on affiliate accounts.
#46

A URL's HTTPS is wrongly based on the request.
https://github.com/copona/copona/blob/master/system/library/url.php#L46-L49

The vulnerable JSON helper has not been removed.
https://github.com/copona/copona/blob/master/system/helper/json.php

Reported XSS vulnerability in admin has not been fixed.

There are probably a few others to add ro this list.

[lucasjkr]

The URL thing was a kludge based on what OC does, which is also stupid.

The solution is simple: people shouldn't run OC or Copona on a server without SSL; however, if they do insist on using it without SSL, then the application shouldn't rewrite URL's to values that the server can't handle.

It's 2018, there are plenty of free certs available, so there's no reason for a site NOT to be using SSL/TLS, let alone one that handles credit cards.

[ADDCreative]

@lucasjkr OpenCart doesn't have the issue (if you set configuration correctly), only Copona. The issue is only a problem if the server is using a certificate, so installing a certificate is not a solution.

Say an attacker is able to see any data that is not encrypted between the client and server. If the client has arrived at the site via a link that was only http://. Maybe generated by the attacker somehow or from a old external or internal link. As all the links on the page are based to the request and not the configuration, any form post actions will be to http:// URLs. If the client is on the login page and login, the login details are sent to the server unencrypted, allowing the attacker to see them.

This would not happen if the URLs were generated based on the configuration (as OpenCart does) rather than the request (as Capona now does).

Yes, a server should be set up correctly redirecting all http:// requests to https:// and using HSTS (if the browser supports it). They maybe a reason this hasn't been done, but there is absolution no reason why Copona shouldn't generate the URLs correctly based on the configuration.

[lucasjkr]

@ADDCreative - The issue has fixed in OC, but was present originally, here is the code I'm talking about, which was what was present when Copona forked, and I'm certain I argued for the solution that OC went with (setting the URL based on config, rather than having PHP try to do it):

https://github.com/opencart/opencart/blob/0ed2675b2bfb4321c2c2fb929fe90e1fac0555d2/upload/system/library/url.php

That's the reason for this kludge in the first place - if you installed OC (or Copona) on a test server without SSL enabled, it would still try to force you over to https urls at some points, even if it wasn't there.

Yes, Rewrite should just look at the URL in the site's config. THat said, there shouldn't be HTTP_SERVER and HTTPS_SERVER's getting defined.

Someone should just import OC's Url.php class and test. It might work as is, or might require replacing a ton of url->rewrite('random url', true) to url->rewrite('random url')

In fact, for the short term, you can take what I rewrote at Opencommerce:
https://github.com/lucasjkr/opencommerce/blob/master/system/library/url.php