Skip to content

Conversation

@ejcx
Copy link

@ejcx ejcx commented Aug 20, 2015

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
Copy link
Author

ejcx commented Aug 20, 2015

@alex-kovac 👍

@alex-kovac
Copy link
Contributor

feels it should go like ;) : @alekseykorzun 👍

@alekseykorzun
Copy link

Haha @alex-kovac

@alekseykorzun
Copy link

👍

@ejcx
Copy link
Author

ejcx commented Aug 21, 2015

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

@alekseykorzun
Copy link

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

@bcoles
Copy link

bcoles commented Jul 26, 2016

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
Copy link
Author

ejcx commented Jul 26, 2016

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants