No supported modules should use file_get_contents to access external URLs

[brynwhyman]

Background

CWP Ops were about to release PHP ini changes that disabled allow_url_fopen and allow_url_include which would prevent the use of file_get_contents() in particular with URLs.

This release had to be postponed because it was discovered some supported modules are still using this function.

Overview

We need to investigate all supported modules (and 3rd party dependancies) to identify where file_get_contents is being used and update it to something more secure.

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

Release considerations

This improvement would focus on new stacks so we would only need to fix it in a new CWP 2 release.

Modules identified so far

• Fulltestsearch: Module shouldn't use file_get_contents to access external URLs silverstripe-fulltextsearch#264
• CWP search: Module shouldn't use file_get_contents to fetch URLs cwp-search#25

[robbieaverill]

I have a feeling that the use of file_get_contents() in project code would also be widespread. I assume some research has been done in this space before the decision to disable it was made?

[brynwhyman]

I assume some research has been done in this space before the decision to disable it was made?

@madmatt, @chillu do you have input here?

[madmatt]

As noted on the CWP announcement, this was only planned to affect new stacks for now. Once the change had been rolled out, further work would be done to find any affected stack code and get it resolved prior to disabling those values for existing stacks.

In addition, CWP developer docs were updated to indicate why it shouldn't be used.

[sminnee]

I believe the ticket title should be "No supported modules should use file_get_contents to access URLs" but I'll let @madmatt confirm that.

[robbieaverill]

I'm finding it hard to justify this as a bug to be honest... but since the announcement calls it a "low impact security risk" then I guess we will

[robbieaverill]

Are we doing this for SS3 as well?

[brynwhyman]

Are we doing this for SS3 as well?

I believe this should focus on new stacks, so it would make sense to target the latest CWP 2 release.