Fix absolute path

[raphaChoquet]

There was conflict with 'assets:install --symlink', which cause the NotLoadableException. It searches in web but the realpath was returned the path in src/.../. That path was outside the defined root path (/web by default).

[alexwilson]

Thanks for contributing!

[alexwilson]

I'm fine with merging this in principle and releasing a patch level, especially as it should fix #846, but this has broken the tests introduced in #775 and I would also prefer to hear from @robfrawley first.

[alexwilson]

Failing tests

[robfrawley]

I'm 👎 on this. Changing

if (!($absolutePath = realpath($this->rootPath.DIRECTORY_SEPARATOR.$path))) {

to

if (!($absolutePath = realpath($this->rootPath).DIRECTORY_SEPARATOR.$path)) {

Removes the ability to ensure the image path is within the root path. Again, you simply have to set a data root in the bundle configuration if you require files outside the default. Also, you might as well change the line to

$absolutePath = $this->rootPath.DIRECTORY_SEPARATOR.$path;

and remove the call to realpath and the if conditional altogether (though this is a horrible idea), as the root path is already resolved at this point (in the constructor).

Moreover, this change breaks already-fixed bugs which would cause /../ in paths to fail (hence the tests that are failing now). Symbolic links are great for many things, but to blindly follow them and allow access to files outside the defined data_root is a security concern that I do not support.

[robfrawley]

Also, can you clarify what you intended to describe @raphaChoquet? The description you gave doesn't make sense to me as src/../ is what should be returned as web lives within this path, once resolved. (As in, src/../web is the correct path for the web root)

It searches in web but the realpath was returned the path in src/.../. That path was outside the defined root path (/web by default).

[raphaChoquet]

I wanted to say i have used the command assets:install with option -symlinks. Then the realpath resolve 'bundles/app/img/my-image.png' by 'src/AppBundle/Resources/public/img/my-image.png'. This cause exception 'Source image invalid "%s" as it is outside of the defined root path'. I suppose we need to define a function to determinate if $path is in the root_path without destroying the symlinks. Perhaps you considerate symlink as a security fail ?

Sorry for my english, i'm not have a good level ^^'

[robfrawley]

@raphaChoquet Don't worry about your English; it's absolutely fine! ;-) I'll spend some time with this over the weekend to see if we can't allow for an array of data roots, or define an alternate realpath function that handles this more gracefully. I get where you are running into problems, but the issue with allowing symbolic links blindly is the ability for unintentionally following routes to other, possibly secure, paths on the entire filesystem. @antoligy Any ideas or guidance on which of the above possible solutions you think may be better to implement?

[robfrawley]

@antoligy @davidromani @adaniloff @raphaChoquet @picks44 Alternatively to all other previously mentioned paths forward, perhaps we should create a realpath resolution toggle via a bundle configuration option? This would allow a user to disable it and ignore the security ramifications of blindly following symbolic links. It should be enabled by default, IMHO, that way people need to explicitly disable it (and understand the ramifications of doing so) before the security check is skipped. Something like:

liip_imagine:
  loaders:
    default:
      filesystem:
        resolve_sym_links: false

[robfrawley]

See #846 (comment) for a possible solution.

[robfrawley]

Closing as this breaks unit tests. Please feel free to re-open a new PR if you can provide an implementation that doesn't break the requirements of this component. I suggest you see if #851 fixes your issue.

[alexwilson]

👍