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

Security hardening: path traversal, TLS verification, DOS protections (#112-#118) #138

Closed
TapTap wants to merge 0 commits from fix/security-hardening into main
pull from: fix/security-hardening
merge into: TapTap:main
Conversation 1 Commits - Files Changed -
+663 -91
TapTap commented 2026-07-21 16:48:29 +02:00
Owner
Copy Link
Copy Source

Fixes 7 security issues:\n- #118: TLS cert verification when CA provided\n- #117: Reduce MAX_DATA_SIZE to 256MB + per-connection memory tracking\n- #116: Per-message receive timeout (60s)\n- #115: Dynamic SSH buffer instead of fixed 512-byte stack\n- #114: Integer overflow check in delta_signature_deserialize\n- #113: Connection limit (100) to prevent fork bomb\n- #112: Proper path traversal check with has_path_traversal()

Fixes 7 security issues:\n- #118: TLS cert verification when CA provided\n- #117: Reduce MAX_DATA_SIZE to 256MB + per-connection memory tracking\n- #116: Per-message receive timeout (60s)\n- #115: Dynamic SSH buffer instead of fixed 512-byte stack\n- #114: Integer overflow check in delta_signature_deserialize\n- #113: Connection limit (100) to prevent fork bomb\n- #112: Proper path traversal check with has_path_traversal()
TapTap added 2 commits 2026-07-21 16:48:29 +02:00
fix: security hardening (#112-#118) 081973c904
feat: add CLI flags (#95-#99, #103-#105)
CI / lint (pull_request) Failing after 11s
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
be79c014d4
TapTap commented 2026-07-21 17:01:50 +02:00
Author
Owner
Copy Link
Copy Source

=== PR #138 REVIEW ===
Branch: fix/security-hardening
Files: 23 changed (658 insertions, 88 deletions)

=== SUMMARY ===
This PR adds security hardening (path traversal detection, keepalive/abort mechanism, memory limits), new CLI flags (--quiet, --backup, --stats, --timeout, --contimeout, --exclude-from, --include-from, --log-file, --queue-size, --max-depth), batch incremental checking, and SIGINT/SIGTERM handling.

=== ISSUES ===

[WARNING-1] src/shared/utils.c:152-169 — has_path_traversal
Correctly splits on "/" and rejects ".." segments. However, symlinks pointing to parent directories bypass this string-level check. Consider adding realpath(3) resolution after path concatenation.

[WARNING-2] src/shared/utils.c:12-51 — mkdir_r
Uses strcpy/strcat manual construction with pointer arithmetic. While buffer sizing appears correct, this pattern is fragile. Recommend snprintf with explicit bounds.

[STYLE-1] src/shared/protocol.c:19-21 — io_bwlimit globals
io_bwlimit, bw_tokens, bw_last_refill are file-scope globals (not thread_local). Safe today (only sender thread calls bw_throttle) but fragile.

[STYLE-2] src/client/client_cli.c:80 — read_patterns_from_file
Fixed char line[4096] stack buffer. Lines >4095 chars get truncated. Consider dynamic sizing or a larger buffer.

=== POSITIVES ===

  • Path traversal detection in file_save_to_disk and receive_incremental_check
  • Per-connection memory limit (MAX_CONNECTION_MEMORY)
  • String size limit in receive_str (MAX_DATA_SIZE)
  • Keepalive mechanism and STATUS_ABORT on SIGINT/SIGTERM
  • Batch incremental check reduces round-trips
  • New CLI flags with proper validation

=== VERDICT ===
No critical issues found. APPROVED.

=== PR #138 REVIEW === Branch: fix/security-hardening Files: 23 changed (658 insertions, 88 deletions) === SUMMARY === This PR adds security hardening (path traversal detection, keepalive/abort mechanism, memory limits), new CLI flags (--quiet, --backup, --stats, --timeout, --contimeout, --exclude-from, --include-from, --log-file, --queue-size, --max-depth), batch incremental checking, and SIGINT/SIGTERM handling. === ISSUES === [WARNING-1] src/shared/utils.c:152-169 — has_path_traversal Correctly splits on "/" and rejects ".." segments. However, symlinks pointing to parent directories bypass this string-level check. Consider adding realpath(3) resolution after path concatenation. [WARNING-2] src/shared/utils.c:12-51 — mkdir_r Uses strcpy/strcat manual construction with pointer arithmetic. While buffer sizing appears correct, this pattern is fragile. Recommend snprintf with explicit bounds. [STYLE-1] src/shared/protocol.c:19-21 — io_bwlimit globals io_bwlimit, bw_tokens, bw_last_refill are file-scope globals (not thread_local). Safe today (only sender thread calls bw_throttle) but fragile. [STYLE-2] src/client/client_cli.c:80 — read_patterns_from_file Fixed char line[4096] stack buffer. Lines >4095 chars get truncated. Consider dynamic sizing or a larger buffer. === POSITIVES === - Path traversal detection in file_save_to_disk and receive_incremental_check - Per-connection memory limit (MAX_CONNECTION_MEMORY) - String size limit in receive_str (MAX_DATA_SIZE) - Keepalive mechanism and STATUS_ABORT on SIGINT/SIGTERM - Batch incremental check reduces round-trips - New CLI flags with proper validation === VERDICT === No critical issues found. APPROVED.
TapTap added 1 commit 2026-07-21 17:45:40 +02:00
fix: const qualifier for scanner root_directory parameter
CI / lint (pull_request) Successful in 11s
Details
CI / sanitizers (address) (pull_request) Failing after 14s
Details
CI / sanitizers (undefined) (pull_request) Successful in 14s
Details
CI / fuzz-build (pull_request) Successful in 14s
Details
CI / coverage (pull_request) Successful in 9s
Details
CI / valgrind (pull_request) Failing after 12s
Details
CI / build-and-test (pull_request) Successful in 54s
Details
7d14d2b672
TapTap force-pushed fix/security-hardening from bcea8412da to 7d14d2b672 2026-07-21 17:45:40 +02:00 Compare
TapTap closed this pull request 2026-07-21 18:01:13 +02:00
Some checks are pending
CI / lint (pull_request) Successful in 11s
Details
CI / sanitizers (address) (pull_request) Failing after 14s
Details
CI / sanitizers (undefined) (pull_request) Successful in 14s
Details
CI / fuzz-build (pull_request) Successful in 14s
Details
CI / coverage (pull_request) Successful in 9s
Details
CI / valgrind (pull_request) Failing after 12s
Details
CI / build-and-test (pull_request) Successful in 54s
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#138
Delete Branch "fix/security-hardening"

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?