Skip to content

fix(security): prevent untarring outside expected path#5279

Merged
jdx merged 1 commit into
mainfrom
settings
Jun 4, 2025
Merged

fix(security): prevent untarring outside expected path#5279
jdx merged 1 commit into
mainfrom
settings

Conversation

@jdx

@jdx jdx commented Jun 4, 2025

Copy link
Copy Markdown
Owner

No description provided.

@github-actions

github-actions Bot commented Jun 4, 2025

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2025.6.0 x -- echo 17.4 ± 0.6 16.7 23.2 1.00
mise x -- echo 17.8 ± 0.6 17.0 23.5 1.02 ± 0.05

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2025.6.0 env 16.6 ± 0.3 16.0 19.1 1.00
mise env 16.7 ± 0.5 16.2 21.8 1.01 ± 0.03

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2025.6.0 hook-env 16.5 ± 0.4 15.9 20.5 1.00
mise hook-env 16.5 ± 0.2 16.1 17.8 1.00 ± 0.03

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2025.6.0 ls 15.1 ± 0.3 14.6 17.3 1.00
mise ls 15.7 ± 0.4 14.8 17.3 1.04 ± 0.04

xtasks/test/perf

Command mise-2025.6.0 mise Variance
install (uncached) 780ms ⚠️ 1135ms -31%
install (cached) 122ms 127ms -3%
ls (uncached) 711ms 718ms +0%
ls (cached) 80ms 81ms -1%
bin-paths (uncached) 720ms ⚠️ 1053ms -31%
bin-paths (cached) 65ms 65ms +0%
task-ls (uncached) 3178ms ⚠️ 3589ms -11%
task-ls (cached) 267ms 256ms +4%

⚠️ Warning: install uncached performance variance is -31%
⚠️ Warning: bin-paths uncached performance variance is -31%
⚠️ Warning: task-ls uncached performance variance is -11%

@jdx jdx enabled auto-merge (squash) June 4, 2025 15:18
@jdx jdx force-pushed the settings branch 3 times, most recently from f8c9294 to 51b63ba Compare June 4, 2025 15:34
@jdx jdx disabled auto-merge June 4, 2025 15:39
@jdx jdx changed the title fix: prevent untarring outside expected path security: prevent untarring outside expected path Jun 4, 2025
@jdx jdx changed the title security: prevent untarring outside expected path fix(security): prevent untarring outside expected path Jun 4, 2025
@jdx jdx enabled auto-merge (squash) June 4, 2025 15:44
@jdx jdx merged commit c988a68 into main Jun 4, 2025
17 checks passed
@jdx jdx deleted the settings branch June 4, 2025 15:53
jdx pushed a commit that referenced this pull request Jun 9, 2025
### 🚀 Features

- **(aqua)** support no_asset and error_message by
[@risu729](https://github.com/risu729) in
[#5303](#5303)
- **(registry)** use ubi backend for func-e by
[@risu729](https://github.com/risu729) in
[#5273](#5273)

### 🐛 Bug Fixes

- **(task)** use empty string for the default value of option by
[@risu729](https://github.com/risu729) in
[#5309](#5309)

### 📚 Documentation

- **(registry)** fix links of registry by
[@risu729](https://github.com/risu729) in
[#5266](#5266)
- **(registry)** fix links to tools by
[@risu729](https://github.com/risu729) in
[#5272](#5272)

### 🧪 Testing

- **(registry)** fix test typos by
[@risu729](https://github.com/risu729) in
[#5269](#5269)

### 🛡️ Security

- **(security)** prevent untarring outside expected path by
[@jdx](https://github.com/jdx) in
[#5279](#5279)
jdx pushed a commit that referenced this pull request Jul 1, 2025
Resolves #5419.

This PR fixes a bug that caused a `No such file or directory` error when
extracting tar archives containing hard links.

### Cause

The issue is caused by
[`tar::Entry::unpack`](https://docs.rs/tar/latest/tar/struct.Entry.html#method.unpack),
which resolves hard link paths relative to the current working directory
instead of the intended destination directory.
When `Entry::unpack` calls the internal `EntryFields::unpack` method, it
passes `None` as the `target_base`, so the hard links are resolved to
the current working directory.

This resulted in `std::fs::hard_link` being called with an incorrect
source path (implementation
[here](https://github.com/alexcrichton/tar-rs/blob/a1c3036af48fa02437909112239f0632e4cfcfae/src/entry.rs#L509)),
leading to the following error:

```
No such file or directory (os error 2) when hard linking ./share/lima/templates/_images/ubuntu-24.04.yaml to /home/ryan/.local/share/mise/installs/lima/1.1.1/share/lima/templates/_images/ubuntu-lts.yaml
```

### Solution

The fix is to replace `Entry::unpack` with
[`Entry::unpack_in`](https://docs.rs/tar/latest/tar/struct.Entry.html#method.unpack_in).

This method correctly uses the destination path as the base for
unpacking contents, including hard links, ensuring that
`std::fs::hard_link` receives the correct source path.

While `Archive::unpack` could also solve this issue, sticking with
`Entry::unpack_in` on a per-entry basis allows us to retain the existing
progress bar functionality.

However, `opts.strip_components` cannot be parsed directly, so I added
logic to move the entries to upper directories.
This might slow down the process, but I couldn't find other solutions
for this.

### Notes

- We might be able to use `std::env::set_current_dir` to change the
current working directory, but that would be a global change and could
cause issues in other parts of the code.

- The use of `Entry::unpack_in` mitigates the security issues that
`absolutize_virtually` was introduced in #5279 to prevent, so it is no
longer necessary.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
andreabedini pushed a commit to andreabedini/mise that referenced this pull request Jul 1, 2025
Resolves jdx#5419.

This PR fixes a bug that caused a `No such file or directory` error when
extracting tar archives containing hard links.

### Cause

The issue is caused by
[`tar::Entry::unpack`](https://docs.rs/tar/latest/tar/struct.Entry.html#method.unpack),
which resolves hard link paths relative to the current working directory
instead of the intended destination directory.
When `Entry::unpack` calls the internal `EntryFields::unpack` method, it
passes `None` as the `target_base`, so the hard links are resolved to
the current working directory.

This resulted in `std::fs::hard_link` being called with an incorrect
source path (implementation
[here](https://github.com/alexcrichton/tar-rs/blob/a1c3036af48fa02437909112239f0632e4cfcfae/src/entry.rs#L509)),
leading to the following error:

```
No such file or directory (os error 2) when hard linking ./share/lima/templates/_images/ubuntu-24.04.yaml to /home/ryan/.local/share/mise/installs/lima/1.1.1/share/lima/templates/_images/ubuntu-lts.yaml
```

### Solution

The fix is to replace `Entry::unpack` with
[`Entry::unpack_in`](https://docs.rs/tar/latest/tar/struct.Entry.html#method.unpack_in).

This method correctly uses the destination path as the base for
unpacking contents, including hard links, ensuring that
`std::fs::hard_link` receives the correct source path.

While `Archive::unpack` could also solve this issue, sticking with
`Entry::unpack_in` on a per-entry basis allows us to retain the existing
progress bar functionality.

However, `opts.strip_components` cannot be parsed directly, so I added
logic to move the entries to upper directories.
This might slow down the process, but I couldn't find other solutions
for this.

### Notes

- We might be able to use `std::env::set_current_dir` to change the
current working directory, but that would be a global change and could
cause issues in other parts of the code.

- The use of `Entry::unpack_in` mitigates the security issues that
`absolutize_virtually` was introduced in jdx#5279 to prevent, so it is no
longer necessary.

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant