security improvements

[curious-rabbit]

Multiple problems affecting security

• Tar symlink/hardlink entries with absolute or ../ paths escape the output directory, because PathBuf::join replaces the base when the second arg is absolute.
• Zip symlink targets aren't checked. Combined with a second entry that writes through the symlink, files land outside the output folder.
• Filenames, zip comments, symlink targets and similar attacker-controlled strings are printed to the terminal with path.display(), so an archive can inject ESC sequences, BiDi overrides (CVE-2021-42574), zero-width chars, etc.
• Setuid/setgid bits from zip entries are preserved on extraction.
• No cap on decompression output, so a small gzip/xz/zstd bomb can fill disk or RAM. Applies to both decompress and list.
• LZMA memory limit is set to u32::MAX (4 GiB) in both decompress.rs and list.rs.
• --password on the command line leaks via ps and shell history; no env var alternative exists.
• Zip extraction creates files with default permissions first, then chmods to the archive mode, leaving a short window where a sensitive file is world-readable.

[tommady]

Hi, thanks for the contribution!

I left a comment.

One small thing I noticed: it looks like LimitedReader went on vacation and didn’t make it into:

1. tar
2. rar

Might be worth inviting it back for consistency (and so things don’t get… too unlimited). 😄

[tommady]

I'd love your validate_dest_inside_root check!
it is a very clever and highly performant way than canonicalization.

@marcospb19 please help check this
thank you

[curious-rabbit]

@marcospb19 could this be merged for the next releases? it does fix several security holes after all.

[curious-rabbit]

rebased to resolve conflicts

[curious-rabbit]

The limiter is now removed again because this may break some use cases

[marcospb19]

@marcospb19 could this be merged for the next releases? it does fix several security holes after all.

I apologize for leaving you hanving.

We had a bug for 6 months where ouch was deleting user files without even requiring any malicious input, ouch was itself always behaving in an evil manner.

With that fixed in 0.8.0, I can now turn my attention into this.

[marcospb19]

I will now solve conflicts and take a thoughtful look.

[marcospb19]

Polite ask, in a next PR please commit these separatedly so I can know what I'm reviewing.

People usually ask for separate PRs but separate commits make it easier too, I tried reviewing this before and had the same trouble, as I keep confusing what every fix is with unrelated code.

[ofek]

Thanks for the work on resolving these security issues! After this is released I plan to include ouch in our developer environments.

[ofek]

Perhaps we can open an issue to track incorporating this in CI https://github.com/ouuan/ZipDiff (not the entire suite of course)