search

silverstripe / silverstripe-fulltextsearch

Adds external full text search engine support to Silverstripe
BSD 3-Clause "New" or "Revised" License
44 stars 83 forks source link

Module shouldn't use file_get_contents to access external URLs #264

Open madmatt opened 5 years ago

madmatt commented 5 years ago

See cross-post issue on silverstripe/cwp-search: https://github.com/silverstripe/cwp-search/issues/25

This module uses file_get_contents() to post/retrieve data from Solr in some instances. It shouldn't do so, as some servers may have allow_url_fopen disabled in php.ini.

Instead, use of Guzzle (or raw curl) is encouraged for security reasons, mainly to prevent accidental remote code execution/remote file inclusion bugs.

Note that this module explicitly isn't susceptible to RFI vulnerabilities as far as I can tell, but if you're trying to use the module on a hardened server this config value is likely disabled.

edit: Also, renaming the variable from $targetDir would help avoid doubt about whether or not it's a URL. Suggested name: $targetUrl

madmatt commented 5 years ago

See search results: https://github.com/silverstripe/silverstripe-fulltextsearch/search?q=file_get_contents&unscoped_q=file_get_contents

madmatt commented 5 years ago

Also note that the underlying library we use uses the Apache_Solr_HttpTransport_FileGetContents HTTP transport by default. This will need to change to use the Apache_Solr_HttpTransport_Curl transport instead. Looks like this should be easy to achieve, but shouldn't be overlooked. (h/t @firesphere for pointing this out)

sminnee commented 5 years ago

Should title be "Module shouldn't use file_get_contents to fetch URLs"?