Bug 606813

Created attachment 425955 [details]
reproducer file

(In reply to comment #0)
> #!/usr/bin/perl
> use Archive::Tar;
> local $Archive::Tar::DEBUG = 0;
> local $Archive::Tar::INSECURE_EXTRACT_MODE  =  0;
> exit 1 if (!Archive::Tar->extract_archive('rep-sym-abspath.tar', 0));
> 
>    where 
> 
> $ tar tvf rep-sym-abspath.tar
> lrwxrwxrwx 0/0       0 2007-09-12 10:59 dir -> /tmp
> -rw------- 0/0       4 2007-09-12 10:59 dir/bar
> 
>    then I got error message "...not allowed under SECURE EXTRACT MODE..."
>    which is right (see [1] for more info) but the script ends up with 
>    zero exit code. Which is IMHO bad. 

Such behaviour is clearly not consistent with Archive::Tar man page.  However, secure mode extract failure is not the only case where you may get such incorrect return value.  Try (as non-root user) e.g.:

cd /etc/
Archive::Tar->extract_archive('/path/to/harmless.tar',0)

>    Additionally I can't find the perl-Archive-Tar component among RHEL6
>    perl components in Bugzilla. Thus filing this bug against the perl 
>    component.

perl-Archive-Tar RPM is now built from perl SRPM.

I'm not sure if the argumentation like "This is a bug. But I won't take care of it as there are many others" is the best resolution ;-)

Anyway I understand it is not critical. So I will not consider it as a show stopper for https://bugzilla.redhat.com/show_bug.cgi?id=295021#c35.

Created attachment 426187 [details]
Possible fix

(In reply to comment #3)
> I'm not sure if the argumentation like "This is a bug. But I won't take care of
> it as there are many others" is the best resolution ;-)

I'm fine with addition of the fix to EL4/EL5 updates too, if we can come up with a good fix.  I've attached patch that should fix extract_archive.  However, I'm not sure whether extract_archive return value as currently defined is all that useful.  What if extraction failure happens in the middle of processing of larger archive (due to insecure path, full disk or permission failure)?  Is it better to return names of files that were extracted already (as you may want to remove them) or just indicate failure?

This fix breaks iter with option extract=>1 (Can't use an undefined value as an ARRAY reference at /path/to/Archive/Tar.pm line 1592).

> Created an attachment (id=426187) [details]
> Possible fix    

In this case I get right exitcode, but loose the error message: "Entry ... is
attempting to extract to a symlinked directory (...). This is considered a
security vulnerability and not allowed under SECURE EXTRACT MODE at ...". 

=> regression.

(In reply to comment #6)
> In this case I get right exitcode, but loose the error message: "Entry ... is
> attempting to extract to a symlinked directory (...). This is considered a
> security vulnerability and not allowed under SECURE EXTRACT MODE at ...". 

What is your test case?  I get error msg printed to stderr and set in $Archive::Tar::error too.

Created attachment 426194 [details]
Updated patch

Work-around for the extract=>1 iter() breakage mentioned above.  Returns empty list on the first extract failure.  Function does not have error return value defined, so the same return value as end-of-archive.

This request was evaluated by Red Hat Product Management for inclusion in a Red
Hat Enterprise Linux major release.  Product Management has requested further
review of this request by Red Hat Engineering, for potential inclusion in a Red
Hat Enterprise Linux Major release.  This request is not yet committed for
inclusion.

> What is your test case?  I get error msg printed to stderr and 
> set in $Archive::Tar::error too.

Basically my test case with reproducer files you can find in [1] but I'm testing it on RHEL6 this way:


-------------8<----------------------------------------------------------

a6# cat repro.pl 
#!/usr/bin/perl

use Archive::Tar;

local $Archive::Tar::DEBUG = 0;
local $Archive::Tar::INSECURE_EXTRACT_MODE  =  0; 

#exit 1 if (!Archive::Tar->extract_archive('some-unexisting-file.tar', 0));
exit 1 if (!Archive::Tar->extract_archive('rep-sym-abspath.tar', 0));





a6# ./repro.pl 
Making symbolic link '/root/dir' to '/tmp' failed at ./repro.pl line 9
Entry 'dir/bar' is attempting to extract to a symlinked directory (/root/dir => /tmp). This is considered a security vulnerability and not allowed under SECURE EXTRACT MODE at ./repro.pl line 9

-------------8<----------------------------------------------------------


[1] https://bugzilla.redhat.com/show_bug.cgi?id=295021#c14

Sorry, I don't see the regression mentioned in comment #6.

Created attachment 426222 [details]
Hmm, I do see it. Please, try out my "patched" file /usr/share/perl5/Archive/Tar.pm.

Note, that I'm using 

a6# rpm -q perl
perl-5.10.1-109.el6.x86_64

$ ./repro.pl ../rep-sym-abspath.tar 
Entry 'dir/bar' is attempting to extract to a symlinked directory (/path/to/dir => /tmp). This is considered a security vulnerability and not allowed under SECURE EXTRACT MODE at ./repro.pl line 8

Error message is printed as expected.

Re: comment #12: My fault. Need to remove previously extracted "dir" first as discussed via IRC. There is no regression in fact. Thanks for clarification.

=> Patch looks acceptable.

(In reply to comment #14)
> Re: comment #12: My fault. Need to remove previously extracted "dir" first as
> discussed via IRC. There is no regression in fact. Thanks for clarification.

The problem here is that _extract_file fails to extract symlink if another file (regular, symlink or directory) with the same name already exists.  Proposed patch causes extract_archive to stop extracting on the first error.

Martin also filed upstream bug:
  https://rt.cpan.org/Public/Bug/Display.html?id=58636

=> VERIFIED

Red Hat Enterprise Linux 6.0 is now available and should resolve
the problem described in this bug report. This report is therefore being closed
with a resolution of CURRENTRELEASE. You may reopen this bug report if the
solution does not work for you.