Received file paths are concatenated directly with the destination root directory without validation. A malicious client can send paths containing .. components (e.g. ../../../etc/cron.d/exploit) and the server will write, read, or delete files outside receive_root_directory.
src/shared/file.c:521-539 — receive_manifest() passes manifest paths to delete_extras(), which also concatenates them with the root.
path_cat() in src/shared/utils.c:127-149 simply joins two strings and normalizes slashes; it does not reject .. components or canonicalize the result. A previous issue (#112) noted a weak strstr(..) check, but the current main branch contains no such check at all.
Server writes to /tmp/recv/../../../tmp/owned = /tmp/owned.
With --delete, an attacker can also delete arbitrary files by listing them in the manifest.
Suggested fix
In path_cat() or a new helper, reject any path component that is exactly ...
Before opening/creating the final path, canonicalize it with realpath() and verify the canonical path is still under receive_root_directory.
Use openat()/mkdirat() relative to a dirfd opened for the root directory to avoid TOCTOU.
Treat absolute incoming paths as errors.
Severity
Critical
Category
security
This issue was automatically generated by the issue-creator agent.
## Description
Received file paths are concatenated directly with the destination root directory without validation. A malicious client can send paths containing `..` components (e.g. `../../../etc/cron.d/exploit`) and the server will write, read, or delete files outside `receive_root_directory`.
Affected call sites:
- `src/shared/file.c:130` — `file_save_to_disk()` uses `path_cat(root_directory, file->path)`.
- `src/shared/file.c:328` — `receive_incremental_check()` uses `path_cat(config->receive_root_directory, check_path)`.
- `src/shared/file.c:521-539` — `receive_manifest()` passes manifest paths to `delete_extras()`, which also concatenates them with the root.
`path_cat()` in `src/shared/utils.c:127-149` simply joins two strings and normalizes slashes; it does **not** reject `..` components or canonicalize the result. A previous issue (#112) noted a weak `strstr(..)` check, but the current `main` branch contains no such check at all.
## Reproduction scenario
1. Start server: `./build/server --dest-dir /tmp/recv --save-to-disk`
2. Malicious client sends `STATUS_NEXT` + `send_str("../../../tmp/owned")` + file data.
3. Server writes to `/tmp/recv/../../../tmp/owned` = `/tmp/owned`.
With `--delete`, an attacker can also delete arbitrary files by listing them in the manifest.
## Suggested fix
1. In `path_cat()` or a new helper, reject any path component that is exactly `..`.
2. Before opening/creating the final path, canonicalize it with `realpath()` and verify the canonical path is still under `receive_root_directory`.
3. Use `openat()`/`mkdirat()` relative to a dirfd opened for the root directory to avoid TOCTOU.
4. Treat absolute incoming paths as errors.
## Severity
Critical
## Category
security
---
_This issue was automatically generated by the issue-creator agent._
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Description
Received file paths are concatenated directly with the destination root directory without validation. A malicious client can send paths containing
..components (e.g.../../../etc/cron.d/exploit) and the server will write, read, or delete files outsidereceive_root_directory.Affected call sites:
src/shared/file.c:130—file_save_to_disk()usespath_cat(root_directory, file->path).src/shared/file.c:328—receive_incremental_check()usespath_cat(config->receive_root_directory, check_path).src/shared/file.c:521-539—receive_manifest()passes manifest paths todelete_extras(), which also concatenates them with the root.path_cat()insrc/shared/utils.c:127-149simply joins two strings and normalizes slashes; it does not reject..components or canonicalize the result. A previous issue (#112) noted a weakstrstr(..)check, but the currentmainbranch contains no such check at all.Reproduction scenario
./build/server --dest-dir /tmp/recv --save-to-diskSTATUS_NEXT+send_str("../../../tmp/owned")+ file data./tmp/recv/../../../tmp/owned=/tmp/owned.With
--delete, an attacker can also delete arbitrary files by listing them in the manifest.Suggested fix
path_cat()or a new helper, reject any path component that is exactly...realpath()and verify the canonical path is still underreceive_root_directory.openat()/mkdirat()relative to a dirfd opened for the root directory to avoid TOCTOU.Severity
Critical
Category
security
This issue was automatically generated by the issue-creator agent.