Closed (fixed)
Project:
S3 File System
Version:
8.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
19 Dec 2019 at 09:59 UTC
Updated:
21 Jun 2020 at 05:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
HbtTundar commentedfor solve this issue a simple solution is that you remove first slash from file name in S3fsStream.php at line 1292 by adding this code
$parts[1] = ltrim($parts[1], '/');
and this should be solved the issue
Comment #2
HbtTundar commentedI don't know how to create a patch for this using composer?
Comment #3
HbtTundar commentedfor solve this issue a simple solution is that you remove first slash from file name in S3fsStream.php at line 1292 by adding this code
$parts[1] = ltrim($parts[1], '/');
and this should be solved the issue
Comment #4
HbtTundar commentedif any body have better solution please create a patch for this
Comment #5
HbtTundar commentedComment #6
rajeevgoleI'm also facing the same issue and confirm #3 works for me.
Comment #7
rajeevgoleComment #8
scrumorg commentedWe are also getting the "Not fully protected" warning after updating Drupal to 8.8.1 from 8.7.11 and can confirm that the patch in #7 resolves it. Would be great to get an official fix in the module though. Thanks!
Comment #9
Sylve commentedThis patch is supposed to help drupal find the .htaccess files that would prevent code execution in an apache server.
Maybe we should address the fact that these files don't need to be uploaded to S3, or if we upload them, maybe we should somehow block public read permissions on these files (which get created for each folder). And this extension seems to upload them to S3 with public read permissions.
Or are we assuming the content of these .htaccess files is openly known and probably not modified by no one?
Comment #10
sophie.skThe patch applied cleanly and did what it said on the tin.
Marking as RTBC, but I'm not sure how to respond to @Sylve's questions. Don't the files just get copied across from the Drupal filesystem? Therefore they should have the same permissions as on Drupal?
Comment #11
darvanenI agree the patch is good, and I think it is a good idea to strip those leading slashes regardless of whether we're dealing with the .htaccess file or not, they have no place here as this method is prepending folders to the path.
As for @Sylve's suggestion that we deal with the fact that S3 doesn't need the the .htaccess, which is correct - I would only do that if we could be 100% certain that the functionality would only ever fire if files were *definitely* being stored in an object store. I think it's safer to make the security test pass because it's finding the file it expects to find.
*Perhaps* there's a bug in core that's appending the leading slashes in a way where they don't belong and that is a more appropriate place to focus efforts? OR perhaps leading slashes are the way paths should be handled, and we should refactor this method to handle them appropriately, I don't know enough about path handling in core to tell.
Comment #12
naveenvalechaCode has been pushed to 8.x-3.x