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
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.
In src/shared/transport_ssh.c:10-14, the RemoteDest struct uses fixed-size buffers:
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_tcheck: 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:
Severity: low