Skip to content

More validation#319

Merged
alexlarsson merged 5 commits intocomposefs:mainfrom
cgwalters:more-validation
Aug 20, 2024
Merged

More validation#319
alexlarsson merged 5 commits intocomposefs:mainfrom
cgwalters:more-validation

Conversation

@cgwalters
Copy link
Copy Markdown
Contributor

mkcomposefs: Fail on an empty symlink target

This one previously ended up with a NULL pointer deference
in the bowels of the EROFS generation.

Signed-off-by: Colin Walters walters@verbum.org


mkcomposefs: Reject . and .. in paths

There's no good reason for us to support this; we should
expect paths to be canonicalized. In theory we could handle
this, but I am doubtful anyone actually relies on it.

In EROFS these are supposed to be "hard links" to the relevant
directories; the EROFS generation adds them if they don't
exist. I tried to do stronger validation at the lcfs_node_*
level but that is trickier.

Let's just reject at the dump file for now.

Signed-off-by: Colin Walters walters@verbum.org


tests: Add a test case that directories can't be hardlinked

Hooray! We were actually validating this already. Just
another corner case I thought of.

Signed-off-by: Colin Walters walters@verbum.org


writer: Also check for dir hardlinks when canonicalizing tree

While we have a check in mkcomposefs.c, let's also have one
at the C API level because we want to guard against misuse/attack
from something directly operating on that API.

Signed-off-by: Colin Walters walters@verbum.org


rust/dumpfile: More validation

  • Also verify path length here
  • And canonicalize and enforce normal form for paths

Signed-off-by: Colin Walters walters@verbum.org


This one previously ended up with a NULL pointer deference
in the bowels of the EROFS generation.

Signed-off-by: Colin Walters <walters@verbum.org>
There's no good reason for us to support this; we should
expect paths to be canonicalized. In theory we *could* handle
this, but I am doubtful anyone actually relies on it.

In EROFS these are supposed to be "hard links" to the relevant
directories; the EROFS generation adds them if they don't
exist. I tried to do stronger validation at the `lcfs_node_*`
level but that is trickier.

Let's just reject at the dump file for now.

Signed-off-by: Colin Walters <walters@verbum.org>
Hooray! We were actually validating this already. Just
another corner case I thought of.

Signed-off-by: Colin Walters <walters@verbum.org>
While we have a check in `mkcomposefs.c`, let's also have one
at the C API level because we want to guard against misuse/attack
from something directly operating on that API.

Signed-off-by: Colin Walters <walters@verbum.org>
Matching the C side, but we want to detect errors where
we can early on the Rust side here too as it's safer.

- Also verify path length here
- Deny hardlinked directories
- And canonicalize and enforce normal form for paths

Signed-off-by: Colin Walters <walters@verbum.org>
@alexlarsson alexlarsson merged commit cf6368c into composefs:main Aug 20, 2024
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.

2 participants