wojodesign / simplecart-js

A simple javascript shopping cart that easily integrates with your current website.
simplecartjs.org
1.79k stars 489 forks source link

Fix file inclusion vulnerability #510

Open ejcx opened 9 years ago

ejcx commented 9 years ago

This file curl allows you to view any file on the system. For example, cloning this repository to the base PHP directory makes a vulnerable page look like this

http://example.com/simplecart-js/test/inc/get-raw-javascript.php?file=file:/etc/passwd

ejcx commented 9 years ago

@alex-kovac 👍

alex-kovac commented 9 years ago

feels it should go like ;) : @AlekseyKorzun 👍

AlekseyKorzun commented 9 years ago

Haha @alex-kovac

AlekseyKorzun commented 9 years ago

:+1:

ejcx commented 9 years ago

Yes wrong person. Sorry @alex-kovac.... @AlekseyKorzun ... I wonder how long these people won't respond. This bug is in hundreds of sites.

AlekseyKorzun commented 9 years ago

@ejcx shoot them an email I think they are working on the next version :dancers:

bcoles commented 8 years ago

This issue affects the QUnit Test Suite bundled with simplecart-js, rather than simplecart-js itself, which may explain - but not excuse - the disinterest shown by the developers.

Concerned users can fix this issue by ensuring the test folder is not accessible via the web server.

Semantically, it's worth noting that this issue relates to file disclosure rather than file inclusion, as it does not lead directly to execution of local files.

Unfortunately, the test suite is also vulnerable to Server-Side Request Forgery (SSRF), which this patch PR does not protect against.

A more secure patch would be:

$urlParts = parse_url($url);
if ($urlParts['scheme'] !== "http" && $urlParts['scheme'] !== "https") exit;

This would limit the impact of SSRF attacks which make use of protocol schemes such as gopher, dict and nntp. However, it would not hinder HTTP(S) SSRF attacks.

Ideally, the test suite should ensure that the requested file is located on the same host on which it's executed, by verifying $urlParts['host'] and $urlParts['port'].

Alternatively, the README should make mention of the dangers associated with leaving the test suite in the web root directory.

Here's some more references regarding the dangers of SSRF.

ejcx commented 8 years ago

Come on @bcoles . It's all semantics. I just want to see the bug fixed. image