You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
// Hmm. Symlinks re-evaluate back to the original file here. Unsure if this is a good thing...finalFilef = FileUtils.resolveFile( dir, entryName );
So when extracting symbolic links in some cases they are resolved to the file the point at instead to their file name. And this turns out to not be a good thing. Let me give you and example to better explain what are the implications.
Lets say we have an archive with symbolic link named symlink that points to regular_file. The call to FileUtils.resolveFile returns the canonical path and the canonical path may be different depending on weather the resolved file exist. Lets say that symlink does not exist yet (the archive is extracted in clean directory). FileUtils.resolveFile will return symlink and latter SymlinkUtils.createSymbolicLink( f, new File( symlinkDestination ) ) will create symbolic named symlink that points to regular_file. So far everything is as expected. But if symlink exists (for example we extract the archive again) then FileUtils.resolveFile will resolve to regular_file (the canonical path). The call to SymlinkUtils.createSymbolicLink will try to create symbolic link named regular_file, but because the file already exists it will do nothing. Although it is not what we expect in most cases still works. If the symbolic link destination didn't changed then the end result will be the expected one. But what if symlink points to file that does not exist. So lets say that symlink already exists on the disk, but regular_file not. Then the call to SymlinkUtils.createSymbolicLink will create symbolik link name regular_file that points to regular_file.
My suggestion is to modify FileUtils.resolveFile so it accepts a flag indicating if we want the canonical path or not. For symbolic link we won't get the canonical path and then f will always point to the symbolic link and not to the target path.
For the security check if the extracted file is outside the destination directory an exception is thrown. I would suggest to keep this check. It prevents extracting symbolic links that point to files outside the destination directory (../some_file for example) if the symbolic link already exists. But this is the current behavior and nobody complained so I would suggest to keep it as it is more secure.
The text was updated successfully, but these errors were encountered:
To quote
AbstractUnArchiver
source code:So when extracting symbolic links in some cases they are resolved to the file the point at instead to their file name. And this turns out to not be a good thing. Let me give you and example to better explain what are the implications.
Lets say we have an archive with symbolic link named
symlink
that points toregular_file
. The call toFileUtils.resolveFile
returns the canonical path and the canonical path may be different depending on weather the resolved file exist. Lets say thatsymlink
does not exist yet (the archive is extracted in clean directory).FileUtils.resolveFile
will returnsymlink
and latterSymlinkUtils.createSymbolicLink( f, new File( symlinkDestination ) )
will create symbolic namedsymlink
that points toregular_file
. So far everything is as expected. But ifsymlink
exists (for example we extract the archive again) thenFileUtils.resolveFile
will resolve toregular_file
(the canonical path). The call toSymlinkUtils.createSymbolicLink
will try to create symbolic link namedregular_file
, but because the file already exists it will do nothing. Although it is not what we expect in most cases still works. If the symbolic link destination didn't changed then the end result will be the expected one. But what ifsymlink
points to file that does not exist. So lets say thatsymlink
already exists on the disk, butregular_file
not. Then the call toSymlinkUtils.createSymbolicLink
will create symbolik link nameregular_file
that points toregular_file
.My suggestion is to modify
FileUtils.resolveFile
so it accepts a flag indicating if we want the canonical path or not. For symbolic link we won't get the canonical path and thenf
will always point to the symbolic link and not to the target path.For the security check if the extracted file is outside the destination directory an exception is thrown. I would suggest to keep this check. It prevents extracting symbolic links that point to files outside the destination directory (
../some_file
for example) if the symbolic link already exists. But this is the current behavior and nobody complained so I would suggest to keep it as it is more secure.The text was updated successfully, but these errors were encountered: