security: SSH transport uses fixed-size buffers in parse_remote_dest #45

New Issue

2026-07-20T14:59:14+02:00

TapTap commented

2026-07-20 14:59:14 +02:00

In src/shared/transport_ssh.c:10-14, the RemoteDest struct uses fixed-size buffers:

typedef struct {
  char user[256];
  char host[256];
  char remote_path[4096];
} RemoteDest;

The parsing function (lines 16-48) checks lengths against these sizes and returns -1 if exceeded. However:

User/host truncation vs. rejection: If a username exceeds 255 characters, the function returns -1 (rejects). But the remote_path check at line 22-23 also returns -1. The behavior is inconsistent with how real SSH handles long paths.
No max_size_t check: The memcpy calls at lines 24, 31, 37, 44 all use runtime-computed lengths that have been bounded, which is correct. But the arbitrary limits (256 for user/host, 4096 for path) may be too restrictive for some environments (e.g., long hostnames in cloud environments, deep directory paths).
snprintf instead of memcpy would be safer: Using memcpy with manual null termination is fragile. snprintf would handle truncation more safely.

Suggested Fix:

Increase buffer sizes or use dynamic allocation
Use snprintf instead of memcpy + manual null termination
Log a warning when truncation occurs instead of silently failing

Severity: low

In src/shared/transport_ssh.c:10-14, the RemoteDest struct uses fixed-size buffers: ```c typedef struct { char user[256]; char host[256]; char remote_path[4096]; } RemoteDest; ``` The parsing function (lines 16-48) checks lengths against these sizes and returns -1 if exceeded. However: 1. **User/host truncation vs. rejection**: If a username exceeds 255 characters, the function returns -1 (rejects). But the remote_path check at line 22-23 also returns -1. The behavior is inconsistent with how real SSH handles long paths. 2. **No `max_size_t` check**: The memcpy calls at lines 24, 31, 37, 44 all use runtime-computed lengths that have been bounded, which is correct. But the arbitrary limits (256 for user/host, 4096 for path) may be too restrictive for some environments (e.g., long hostnames in cloud environments, deep directory paths). 3. **snprintf instead of memcpy would be safer**: Using memcpy with manual null termination is fragile. snprintf would handle truncation more safely. Suggested Fix: - Increase buffer sizes or use dynamic allocation - Use snprintf instead of memcpy + manual null termination - Log a warning when truncation occurs instead of silently failing Severity: low

TapTap referenced this issue from a commit

2026-07-20 17:09:16 +02:00

Fix 11 Gitea issues (#29, #30, #31, #35, #38, #41, #42, #43, #44, #45, #46)

TapTap referenced this issue

2026-07-20 17:13:19 +02:00

Fix 11 Gitea issues — bugs, quality, security, and refactoring #47

TapTap closed this issue

2026-07-20 17:51:46 +02:00

Sign in to join this conversation.