Directory traversal in bsdcpio?

[Alexander Cherepanov]

Hi!

libarchive seems to be quite cautious in various regards and bsdtar does
guard against directory traversals by default. Cool!

bsdcpio, by default, rejects .. in file names and doesn't go through
symlinks but happily extracts files using absolute paths:

$ touch /tmp/abs
$ echo /tmp/abs | ./bsdcpio -ov > test.cpio
/tmp/abs
1 block
$ rm /tmp/abs
$ ./bsdcpio -iv < test.cpio
/tmp/abs
1 block
$ ls /tmp/abs
/tmp/abs

Is it just an oversight or there are some reasons for this?

And a more general question. AIUI FreeBSD switched to libarchive for
their tar(1) and cpio(1) but not pax(1) while OpenBSD didn't. It is
still using original (for some value of "original":-) tar/cpio/pax which
doesn't protect from directory traversals. And it's not considered a
vulnerability in BSD world? Links to previous discussions are welcome.

--
Alexander Cherepanov

[Tim Kientzle]

> On Jan 9, 2015, at 5:50 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
>
> Hi!
>
> libarchive seems to be quite cautious in various regards and bsdtar does guard against directory traversals by default. Cool!
>
> bsdcpio, by default, rejects .. in file names and doesn't go through symlinks but happily extracts files using absolute paths:
>
> $ touch /tmp/abs
> $ echo /tmp/abs | ./bsdcpio -ov > test.cpio
> /tmp/abs
> 1 block
> $ rm /tmp/abs
> $ ./bsdcpio -iv < test.cpio
> /tmp/abs
> 1 block
> $ ls /tmp/abs
> /tmp/abs
>
> Is it just an oversight or there are some reasons for this?

Just an oversight.

Patches to correct this would be much appreciated.

Cheers,

Tim

[Alexander Cherepanov]

The proper fix probably involves adding a new flag in libarchive or
renaming/retargeting ARCHIVE_EXTRACT_SECURE_NODOTDOT. The interaction of
this with interactive renaming in bsdcpio is not entirely clear: right
now bsdcpio rejects names with '..' even if it's entered interactively.
Not intuitive but I'm not sure if it's wrong or not. But I digressed...

Quick-n-dirty patch to bsdcpio is attached.

--
Alexander Cherepanov

[Tim Kientzle]

> The proper fix probably involves adding a new flag in libarchive or renaming/retargeting ARCHIVE_EXTRACT_SECURE_NODOTDOT. The interaction of this with interactive renaming in bsdcpio is not entirely clear: right now bsdcpio rejects names with '..' even if it's entered interactively. Not intuitive but I'm not sure if it's wrong or not. But I digressed…

I’ll take a look at your patch this week.

In bsdtar, this is handled as part of the renaming processing, and that would certainly be one way to add it to bsdcpio.

But I think other libarchive users would benefit more from a much simpler approach:

* Add a new flag ARCHIVE_EXTRACT_SECURE_NO_ABSOLUTE_PATHS
* modify archive_write_disk to simply reject any path that is an absolute path.

This could easily be enabled by default in bsdcpio and other clients.

Tim

>
> Quick-n-dirty patch to bsdcpio is attached.
>
> --
> Alexander Cherepanov
>

> --
> You received this message because you are subscribed to the Google Groups "libarchive-discuss" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to libarchive-disc...@googlegroups.com.
> To post to this group, send email to libarchiv...@googlegroups.com.
> Visit this group at http://groups.google.com/group/libarchive-discuss.
> For more options, visit https://groups.google.com/d/optout.
> <0001-Quick-n-dirty-fix-for-bsdcpio-directory-traversal-vu.patch>

[Tim Kientzle]

> On Jan 11, 2015, at 9:24 PM, Tim Kientzle <t...@kientzle.com> wrote:
>
>>
>> On Jan 11, 2015, at 9:15 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
>>
>> On 2015-01-10 08:33, Tim Kientzle wrote:
>>>> On Jan 9, 2015, at 5:50 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
>>>> libarchive seems to be quite cautious in various regards and bsdtar does guard against directory traversals by default. Cool!
>>>>
>>>> bsdcpio, by default, rejects .. in file names and doesn't go through symlinks but happily extracts files using absolute paths:

The attached patch shows how I would like to handle this. Note that it implements this as a new feature within libarchive that other users may appreciate as well.

I may not have time to work on this further for a few weeks; if you have time to finish it up (including writing unit tests for libarchive and/or cpio to verify that this option actually works), I would appreciate the help.

Cheers,

Tim

[Alexander Cherepanov]

On 2015-01-12 08:24, Tim Kientzle wrote:
>
>> On Jan 11, 2015, at 9:15 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
>>
>> On 2015-01-10 08:33, Tim Kientzle wrote:
>>>> On Jan 9, 2015, at 5:50 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
>>>> libarchive seems to be quite cautious in various regards and bsdtar does guard against directory traversals by default. Cool!
>>>>
>>>> bsdcpio, by default, rejects .. in file names and doesn't go through symlinks but happily extracts files using absolute paths:
>>>>
>>>> $ touch /tmp/abs
>>>> $ echo /tmp/abs | ./bsdcpio -ov > test.cpio
>>>> /tmp/abs
>>>> 1 block
>>>> $ rm /tmp/abs
>>>> $ ./bsdcpio -iv < test.cpio
>>>> /tmp/abs
>>>> 1 block
>>>> $ ls /tmp/abs
>>>> /tmp/abs
>>>>
>>>> Is it just an oversight or there are some reasons for this?
>>>
>>> Just an oversight.
>>>
>>> Patches to correct this would be much appreciated.
>>
>> The proper fix probably involves adding a new flag in libarchive or renaming/retargeting ARCHIVE_EXTRACT_SECURE_NODOTDOT. The interaction of this with interactive renaming in bsdcpio is not entirely clear: right now bsdcpio rejects names with '..' even if it's entered interactively. Not intuitive but I'm not sure if it's wrong or not. But I digressed…
>
> I’ll take a look at your patch this week.

Thanks.

> In bsdtar, this is handled as part of the renaming processing, and that would certainly be one way to add it to bsdcpio.

I've started from looking into bsdtar but it seems to be more
complicated in this regard. Then I found a comment "TODO: Publish the
path normalization routines in libarchive[...]" and figured that a
proper fix could be quite involving.

> But I think other libarchive users would benefit more from a much simpler approach:
>
> * Add a new flag ARCHIVE_EXTRACT_SECURE_NO_ABSOLUTE_PATHS
> * modify archive_write_disk to simply reject any path that is an absolute path.
>
> This could easily be enabled by default in bsdcpio and other clients.

Perhaps it's possible to fix it at the level of libarchive instead of
bsdcpio but without full cleaning of bsdtar. I'm not sure what it will take.

OTOH fixing bsdcpio is a small isolated undertaking. Perhaps some folks
will want to backport this fix to previous versions of libarchive&co.
And for a security fix, the smaller the better. So I would appreciate if
you can take a look at my patch. It is surely not ideal -- it's probably
is very not useful on Windows. But it would be nice to know that it
fixes the issue at least on Linux/BSD.

--
Alexander Cherepanov

[Alexander Cherepanov]

I may be able to look into it next week, I'm not sure. But:

- it's not clear to me what path_is_absolute (referenced in your patch)
should do exactly. E.g. bsdtar remove leading drive letter even on
Linux. IMHO this is strange;

- is non-windows-only version is ok?

--
Alexander Cherepanov

[Tim Kientzle]

On Jan 15, 2015, at 12:22 PM, Alexander Cherepanov <cher...@mccme.ru> wrote:

On 2015-01-13 08:16, Tim Kientzle wrote:

On Jan 11, 2015, at 9:24 PM, Tim Kientzle <t...@kientzle.com> wrote:

On Jan 11, 2015, at 9:15 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
On 2015-01-10 08:33, Tim Kientzle wrote:

On Jan 9, 2015, at 5:50 AM, Alexander Cherepanov <cher...@mccme.ru> wrote:
libarchive seems to be quite cautious in various regards and bsdtar does guard against directory traversals by default. Cool!

bsdcpio, by default, rejects .. in file names and doesn't go through symlinks but happily extracts files using absolute paths:

The attached patch shows how I would like to handle this.  Note that it implements this as a new feature within libarchive that other users may appreciate as well.

I may not have time to work on this further for a few weeks; if you have time to finish it up (including writing unit tests for libarchive and/or cpio to verify that this option actually works), I would appreciate the help.

I may be able to look into it next week, I'm not sure. But:

- it's not clear to me what  path_is_absolute (referenced in your patch) should do exactly.

I’m not yet entirely sure.

E.g. bsdtar remove leading drive letter even on Linux. IMHO this is strange;

For bsdtar, I think this is probably the right behavior:  I think that something archived as A:/somefile on a Windows system should in fact probably extract to ./somefile by default on Linux, not as ./A:/somefile

For this application, where we’re just flagging dangerous paths, it might make more sense to use different rules depending on the current system.

- is non-windows-only version is ok?

To start with, sure.  One reason I deliberately wanted that check in a separate function is so we can get help from different people to figure out what it should do on different platforms.

Cheers,

Tim