Re: [Bug-tar] [PATCH] Option to reject files similar to tarbombs

[Connor Behan]

On 21/09/13 08:28 AM, Paul Eggert wrote:
> A few comments:
>
> It shouldn't be necessary to call malloc or to copy the
> file name except in the special case where the first
> top level is found.
>
> Don't store string lengths in 'int'.  Use size_t,
> in case the string length exceeds INT_MAX.
>
> Tar already prevents users from misusing "..", so
> why does this code do that as well?  How does it
> interact with existing ".." code?
Thanks. I missed this before because ".." replacements are in paxlib.
Unless the user passes --absolute-names (which should conflict with this
new option) a file name transformation is applied before
extract_archive(). This means the handling of ".." and leading slashes
in my patch is dead code.
> I don't understand why the code is worried about ".".
The name transformation doesn't remove "." so even without the
--absolute-names option, archives can have names like "././././foo/bar".
Checking for "." only happens so that it knows the top-level name is
before the fifth slash, not before the first slash.
> The documentation makes it sound like if the archive
> contains names like "./foo1/x" and "./foo1/y", then
> that's an error, because the "." is duplicated.
> If that's intended, it should be stated more clearly.
I'll try to make this more clear. The above names should not produce an
error and they don't with the patch applied.
> The documentation should state how the new option interacts
> with the --strip-components, --transform, and --absolute-file-names
> options.  That is, do we insist on a single top level before or
> after these other options transform the file name?  Also,
> please look at all the other options and see if there are
> other interactions that I've missed.
I will write that the insistence on a single top-level is done after
--strip-components and --transform. I think it makes sense to not allow
--absolute-names with --one-top-level because these would just cancel
each other out and require more code.
>
> The option name should start with --one, not with --single,
> by analogy with --one-file-system.

signature.asc
Description: OpenPGP digital signature