Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

symlinks not checked during alloc migration #19887

Closed
tgross opened this issue Feb 6, 2024 · 0 comments
Closed

symlinks not checked during alloc migration #19887

tgross opened this issue Feb 6, 2024 · 0 comments

Comments

@tgross
Copy link
Member

tgross commented Feb 6, 2024

A bug was fixed in allocation directory migration. The Nomad client did not check that any symlinks in the archive pointed outside the allocation directory. While task driver sandboxing will protect against processes inside the task from reading/writing through the symlink, this doesn't protect against the client itself from performing unintended operations outside the sandbox, such as the template-based attack described in #19888

@tgross tgross added the type/bug label Feb 6, 2024
@tgross tgross added this to the 1.7.x milestone Feb 6, 2024
@tgross tgross self-assigned this Feb 6, 2024
@tgross tgross changed the title (placeholder) symlinks not checked during alloc migration Feb 8, 2024
@tgross tgross closed this as completed Feb 8, 2024
tgross added a commit that referenced this issue Feb 8, 2024
During allocation directory migration, the client was not checking that any
symlinks in the archive aren't pointing to somewhere outside the allocation
directory. While task driver sandboxing will protect against processes inside
the task from reading/writing thru the symlink, this doesn't protect against the
client itself from performing unintended operations outside the sandbox.

This changeset includes two changes:

* Update the archive unpacking to check the source of symlinks and require that
  they fall within the sandbox.
* Fix a bug in the symlink check where it was using `filepath.Rel` which doesn't
  work for paths in the sibling directories of the sandbox directory. This bug
  doesn't appear to be exploitable but caused errors in testing.

Fixes: #19887
nvanthao pushed a commit to nvanthao/nomad that referenced this issue Mar 1, 2024
During allocation directory migration, the client was not checking that any
symlinks in the archive aren't pointing to somewhere outside the allocation
directory. While task driver sandboxing will protect against processes inside
the task from reading/writing thru the symlink, this doesn't protect against the
client itself from performing unintended operations outside the sandbox.

This changeset includes two changes:

* Update the archive unpacking to check the source of symlinks and require that
  they fall within the sandbox.
* Fix a bug in the symlink check where it was using `filepath.Rel` which doesn't
  work for paths in the sibling directories of the sandbox directory. This bug
  doesn't appear to be exploitable but caused errors in testing.

Fixes: hashicorp#19887
nvanthao pushed a commit to nvanthao/nomad that referenced this issue Mar 1, 2024
During allocation directory migration, the client was not checking that any
symlinks in the archive aren't pointing to somewhere outside the allocation
directory. While task driver sandboxing will protect against processes inside
the task from reading/writing thru the symlink, this doesn't protect against the
client itself from performing unintended operations outside the sandbox.

This changeset includes two changes:

* Update the archive unpacking to check the source of symlinks and require that
  they fall within the sandbox.
* Fix a bug in the symlink check where it was using `filepath.Rel` which doesn't
  work for paths in the sibling directories of the sandbox directory. This bug
  doesn't appear to be exploitable but caused errors in testing.

Fixes: hashicorp#19887
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant