TapTap/FastSync
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: resolve all security issues (#154, #156, #157, #159, #160, #161, #162, #170) #172

Closed
TapTap wants to merge 0 commits from fix/security-all into main
pull from: fix/security-all
merge into: TapTap:main
Conversation 0 Commits - Files Changed -
+143 -26
TapTap commented 2026-07-29 18:52:26 +02:00
Owner
Copy Link
Copy Source

Security Fixes

This PR fixes 8 security issues across the codebase:

#154: Path traversal prevents writes/deletes outside destination root

  • Added realpath() resolution in file_save_to_disk() to prevent symlink-based escapes
  • Changed delete_extras_walk() to use lstat() instead of stat() and skip symlinks
  • Changed receive_incremental_check() and receive_files() CHECK_BATCH to use lstat()

#156: Unbounded wire sizes enable OOM denial of service

  • Capped strings at 64 KB in receive_str()
  • Capped data payloads at 100 MB in receive_data()

#157: Metadata restoration applies arbitrary mode bits including setuid/setgid

  • Already fixed: code strips S_ISUID and S_ISGID bits in file_restore_metadata()

#159: zstd decompression may allocate ZSTD_CONTENTSIZE_UNKNOWN bytes

  • Added check for ZSTD_CONTENTSIZE_UNKNOWN; fall back to conservative (3x compressed) size

#160: TLS I/O doesn't handle SSL_ERROR_WANT_READ / SSL_ERROR_WANT_WRITE

  • Fixed SSL_accept()/SSL_connect() retry loop in wrap_fd_with_ssl()
  • (send_n_data/receive_n_data already handled these correctly)

#161: TLS client lacks hostname verification

  • Added SSL_set1_host() call before SSL_connect() to enable hostname verification
  • Updated test certificates to include IP SAN 127.0.0.1

#162: --delete follows symlinks and may delete outside destination tree

  • Changed delete_extras_walk() to use lstat() and skip symlinks

#170: Delta deserialization doesn't validate array counts from network

  • Added maximum block count (1M) and instruction count (1M) validation in delta deserialization

Files Changed (9 files, +143/-26)

  • src/shared/utils.c - lstat + skip symlinks in delete_extras_walk
  • src/shared/file.c - realpath validation in file_save_to_disk, lstat in receive_incremental_check
  • src/shared/protocol.c - bounded string/data sizes
  • src/shared/protocol.h - MAX_STRING_SIZE/MAX_DATA_PAYLOAD_SIZE
  • src/shared/compression.c - ZSTD_CONTENTSIZE_UNKNOWN check
  • src/shared/transport_tls.c - SSL WANT_READ/WANT_WRITE retry, SSL_set1_host hostname verification
  • src/shared/delta.c - max block/instruction count validation
  • src/server/server.c - lstat in CHECK_BATCH path
  • tests/integration/test_tls.py - IP SAN in test certificates
## Security Fixes This PR fixes 8 security issues across the codebase: ### #154: Path traversal prevents writes/deletes outside destination root - Added `realpath()` resolution in `file_save_to_disk()` to prevent symlink-based escapes - Changed `delete_extras_walk()` to use `lstat()` instead of `stat()` and skip symlinks - Changed `receive_incremental_check()` and `receive_files()` CHECK_BATCH to use `lstat()` ### #156: Unbounded wire sizes enable OOM denial of service - Capped strings at 64 KB in `receive_str()` - Capped data payloads at 100 MB in `receive_data()` ### #157: Metadata restoration applies arbitrary mode bits including setuid/setgid - Already fixed: code strips S_ISUID and S_ISGID bits in `file_restore_metadata()` ### #159: zstd decompression may allocate ZSTD_CONTENTSIZE_UNKNOWN bytes - Added check for `ZSTD_CONTENTSIZE_UNKNOWN`; fall back to conservative (3x compressed) size ### #160: TLS I/O doesn't handle SSL_ERROR_WANT_READ / SSL_ERROR_WANT_WRITE - Fixed `SSL_accept()`/`SSL_connect()` retry loop in `wrap_fd_with_ssl()` - (send_n_data/receive_n_data already handled these correctly) ### #161: TLS client lacks hostname verification - Added `SSL_set1_host()` call before `SSL_connect()` to enable hostname verification - Updated test certificates to include IP SAN 127.0.0.1 ### #162: --delete follows symlinks and may delete outside destination tree - Changed `delete_extras_walk()` to use `lstat()` and skip symlinks ### #170: Delta deserialization doesn't validate array counts from network - Added maximum block count (1M) and instruction count (1M) validation in delta deserialization ## Files Changed (9 files, +143/-26) - `src/shared/utils.c` - lstat + skip symlinks in delete_extras_walk - `src/shared/file.c` - realpath validation in file_save_to_disk, lstat in receive_incremental_check - `src/shared/protocol.c` - bounded string/data sizes - `src/shared/protocol.h` - MAX_STRING_SIZE/MAX_DATA_PAYLOAD_SIZE - `src/shared/compression.c` - ZSTD_CONTENTSIZE_UNKNOWN check - `src/shared/transport_tls.c` - SSL WANT_READ/WANT_WRITE retry, SSL_set1_host hostname verification - `src/shared/delta.c` - max block/instruction count validation - `src/server/server.c` - lstat in CHECK_BATCH path - `tests/integration/test_tls.py` - IP SAN in test certificates
TapTap added 1 commit 2026-07-29 18:52:27 +02:00
fix: resolve all security issues (#154, #156, #157, #159, #160, #161, #162, #170)
CI / lint (pull_request) Failing after 3s
Details
CI / build-and-test (pull_request) Has been skipped
Details
CI / sanitizers (address) (pull_request) Has been skipped
Details
CI / sanitizers (undefined) (pull_request) Has been skipped
Details
CI / fuzz-build (pull_request) Has been skipped
Details
CI / coverage (pull_request) Has been skipped
Details
CI / valgrind (pull_request) Has been skipped
Details
69773bd16a
TapTap closed this pull request 2026-07-29 19:25:05 +02:00
Some checks are pending
CI / lint (pull_request) Failing after 3s
Details
CI / build-and-test (pull_request) Has been skipped
Details
CI / sanitizers (address) (pull_request) Has been skipped
Details
CI / sanitizers (undefined) (pull_request) Has been skipped
Details
CI / fuzz-build (pull_request) Has been skipped
Details
CI / coverage (pull_request) Has been skipped
Details
CI / valgrind (pull_request) Has been skipped
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
bug
Defect or bug
 enhancement
Feature or improvement
 memory
Memory safety issue
 needs-triage
Needs human review
 performance
Performance issue
 quality
Code quality improvement
 security
Security vulnerability
 threading
Threading or concurrency
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
TapTap
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: TapTap/FastSync#172
Delete Branch "fix/security-all"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?