You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joseph Wu <jo...@mesosphere.io> on 2019/03/06 19:35:08 UTC
Review Request 70142: Added ARCHIVE_EXTRACT_SECURE_NODOTDOT flag to
archiver default.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70142/
-----------------------------------------------------------
Review request for mesos, Andrei Budnik, Gilbert Song, and Greg Mann.
Bugs: MESOS-9610
https://issues.apache.org/jira/browse/MESOS-9610
Repository: mesos
Description
-------
This enables a security flag provided by libarchive, which disallows
extraction of archives that contain '..' in hardlinks or files.
Without this flag, it is possible to provide the archiver with
an archive and overwrite arbitrary files in the user's parent directory
or further up.
Diffs
-----
3rdparty/stout/include/stout/archiver.hpp 2447797ee05f48ab6d8b046d862aede8dec36bea
3rdparty/stout/tests/archiver_tests.cpp cdf24a5d9accb1082e8bf3809c865a92d93e63d8
Diff: https://reviews.apache.org/r/70142/diff/1/
Testing
-------
```
cmake --build . --target stout-tests
3rdparty/stout/tests/stout-tests --gtest_filter="*Archiver*"
```
Thanks,
Joseph Wu
Re: Review Request 70142: Added ARCHIVE_EXTRACT_SECURE_NODOTDOT flag
to archiver default.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70142/#review213510
-----------------------------------------------------------
Patch looks great!
Reviews applied: [70142]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers --disable-parallel-test-execution' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On March 6, 2019, 11:35 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70142/
> -----------------------------------------------------------
>
> (Updated March 6, 2019, 11:35 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, and Greg Mann.
>
>
> Bugs: MESOS-9610
> https://issues.apache.org/jira/browse/MESOS-9610
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This enables a security flag provided by libarchive, which disallows
> extraction of archives that contain '..' in hardlinks or files.
> Without this flag, it is possible to provide the archiver with
> an archive and overwrite arbitrary files in the user's parent directory
> or further up.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/archiver.hpp 2447797ee05f48ab6d8b046d862aede8dec36bea
> 3rdparty/stout/tests/archiver_tests.cpp cdf24a5d9accb1082e8bf3809c865a92d93e63d8
>
>
> Diff: https://reviews.apache.org/r/70142/diff/1/
>
>
> Testing
> -------
>
> ```
> cmake --build . --target stout-tests
> 3rdparty/stout/tests/stout-tests --gtest_filter="*Archiver*"
> ```
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70142: Added ARCHIVE_EXTRACT_SECURE_NODOTDOT flag
to archiver default.
Posted by Joseph Wu <jo...@mesosphere.io>.
> On March 7, 2019, 6:12 a.m., Andrei Budnik wrote:
> > 3rdparty/stout/include/stout/archiver.hpp
> > Line 43 (original), 43 (patched)
> > <https://reviews.apache.org/r/70142/diff/1/?file=2129220#file2129220line43>
> >
> > `libarchive` supports the
> > `ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS` flag:
> > https://github.com/libarchive/libarchive/blob/f77e06a338a9b5414444de406da5a03f0bda8c00/libarchive/archive.h#L692-L693
> >
> > Should we use this flag by default as well?
We should.
I'll write a separate patch to add this however, since the unit test will (probably) exercise a different exploit.
- Joseph
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70142/#review213518
-----------------------------------------------------------
On March 6, 2019, 11:35 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70142/
> -----------------------------------------------------------
>
> (Updated March 6, 2019, 11:35 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, and Greg Mann.
>
>
> Bugs: MESOS-9610
> https://issues.apache.org/jira/browse/MESOS-9610
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This enables a security flag provided by libarchive, which disallows
> extraction of archives that contain '..' in hardlinks or files.
> Without this flag, it is possible to provide the archiver with
> an archive and overwrite arbitrary files in the user's parent directory
> or further up.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/archiver.hpp 2447797ee05f48ab6d8b046d862aede8dec36bea
> 3rdparty/stout/tests/archiver_tests.cpp cdf24a5d9accb1082e8bf3809c865a92d93e63d8
>
>
> Diff: https://reviews.apache.org/r/70142/diff/1/
>
>
> Testing
> -------
>
> ```
> cmake --build . --target stout-tests
> 3rdparty/stout/tests/stout-tests --gtest_filter="*Archiver*"
> ```
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70142: Added ARCHIVE_EXTRACT_SECURE_NODOTDOT flag
to archiver default.
Posted by Andrei Budnik <ab...@mesosphere.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70142/#review213518
-----------------------------------------------------------
3rdparty/stout/include/stout/archiver.hpp
Line 43 (original), 43 (patched)
<https://reviews.apache.org/r/70142/#comment299465>
`libarchive` supports the
`ARCHIVE_EXTRACT_SECURE_NOABSOLUTEPATHS` flag:
https://github.com/libarchive/libarchive/blob/f77e06a338a9b5414444de406da5a03f0bda8c00/libarchive/archive.h#L692-L693
Should we use this flag by default as well?
- Andrei Budnik
On March 6, 2019, 7:35 p.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70142/
> -----------------------------------------------------------
>
> (Updated March 6, 2019, 7:35 p.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, and Greg Mann.
>
>
> Bugs: MESOS-9610
> https://issues.apache.org/jira/browse/MESOS-9610
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This enables a security flag provided by libarchive, which disallows
> extraction of archives that contain '..' in hardlinks or files.
> Without this flag, it is possible to provide the archiver with
> an archive and overwrite arbitrary files in the user's parent directory
> or further up.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/archiver.hpp 2447797ee05f48ab6d8b046d862aede8dec36bea
> 3rdparty/stout/tests/archiver_tests.cpp cdf24a5d9accb1082e8bf3809c865a92d93e63d8
>
>
> Diff: https://reviews.apache.org/r/70142/diff/1/
>
>
> Testing
> -------
>
> ```
> cmake --build . --target stout-tests
> 3rdparty/stout/tests/stout-tests --gtest_filter="*Archiver*"
> ```
>
>
> Thanks,
>
> Joseph Wu
>
>
Re: Review Request 70142: Added ARCHIVE_EXTRACT_SECURE_NODOTDOT flag
to archiver default.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70142/#review213537
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On March 6, 2019, 11:35 a.m., Joseph Wu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70142/
> -----------------------------------------------------------
>
> (Updated March 6, 2019, 11:35 a.m.)
>
>
> Review request for mesos, Andrei Budnik, Gilbert Song, and Greg Mann.
>
>
> Bugs: MESOS-9610
> https://issues.apache.org/jira/browse/MESOS-9610
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This enables a security flag provided by libarchive, which disallows
> extraction of archives that contain '..' in hardlinks or files.
> Without this flag, it is possible to provide the archiver with
> an archive and overwrite arbitrary files in the user's parent directory
> or further up.
>
>
> Diffs
> -----
>
> 3rdparty/stout/include/stout/archiver.hpp 2447797ee05f48ab6d8b046d862aede8dec36bea
> 3rdparty/stout/tests/archiver_tests.cpp cdf24a5d9accb1082e8bf3809c865a92d93e63d8
>
>
> Diff: https://reviews.apache.org/r/70142/diff/1/
>
>
> Testing
> -------
>
> ```
> cmake --build . --target stout-tests
> 3rdparty/stout/tests/stout-tests --gtest_filter="*Archiver*"
> ```
>
>
> Thanks,
>
> Joseph Wu
>
>