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: :main

:dev

:main

:feat/ignore-existing

:feat/chmod

:feat/existing

:docs/rsync-compatible-readme-clean

:docs/rsync-compatible-readme

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) 69773bd16a

TapTap closed this pull request 2026-07-29 19:25:05 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

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 labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

TapTap

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TapTap/FastSync#172