Devel #1731
Devel #1731
Conversation
…eadpassphrase When running on iOS, the standard input might be set to non-blocking mode by the runtime or other components. This caused `readpassphrase` to fail immediately with EAGAIN (interpreted as empty input), resulting in authentication failure without a prompt. This change modifies `readpassphrase` to: - Check if the input FD has `O_NONBLOCK` set. - If so, temporarily clear the flag to allow blocking read. - Restore the original flag after reading or before returning. This ensures the user is prompted for a password as expected.
…readpassphrase The ssh client on iOS runs in a threaded environment where stdin (STDIN_FILENO) is often set to non-blocking mode (O_NONBLOCK) by the runtime or other components (e.g., to support non-blocking network I/O in the shared process). However, `readpassphrase` relies on blocking `read()` to wait for user input. When `read()` returns `EAGAIN` (due to O_NONBLOCK), `readpassphrase` interprets this as an error/EOF and returns an empty string or NULL immediately. This caused the ssh client to loop through authentication attempts sending empty passwords without ever prompting the user. This change modifies `openbsd-compat/readpassphrase.c` (guarded by `PSCAL_TARGET_IOS`) to: 1. Check if the input file descriptor has `O_NONBLOCK` set. 2. Temporarily clear `O_NONBLOCK` before entering the read loop. 3. Restore the original flags before returning. This ensures the password prompt appears and waits for input as expected.
Fix ssh password prompt failure on iOS
Fix ssh password prompt skipping on iOS by enforcing blocking IO in readpassphrase
Apply pending per-session winsize updates with SIGWINCH Guard input queue access and output ring routing Harden terminal input buffering and nextvi output handling
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return false; | ||
| } | ||
| fprintf(stderr, "[builtin-missing] name=%s canonical=%s\n", | ||
| name ? name : "<null>", |
Check failure
Code scanning / CodeQL
Redundant null check due to previous dereference High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
In general, this pattern is fixed by either (a) checking the pointer for null before the first dereference and handling the null case appropriately, or (b) if the pointer is guaranteed to be non-null by design, removing later redundant null checks and documenting the assumption. Here, the function already has an early guard on cmd and cmd->argc, but not on name. To keep behavior robust without changing semantics, the safest approach is to add a null/empty-string check for name before it is dereferenced, and then rely on name being non-null afterward.
Concretely, in shellInvokeBuiltin we should adjust the initial checks so that name is validated (null or empty) before it is used. One clean way is to move the const char *name = cmd->argv[0]; assignment immediately after the cmd/argc guard and then add a combined condition that returns false if name is NULL or points to an empty string, before the later use at line 2516. Then the separate if (!name || *name == '\0') block becomes unnecessary and can be removed. This ensures that by the time we reach the fprintf at 2567–2569, name is guaranteed non-null, making the ternary name ? name : "<null>" redundant; we can safely simplify it to just name. This resolves the “redundant null check due to previous dereference” because there will no longer be any meaningful null check on name after its first guaranteed-safe dereference.
Because we must only modify shown code in src/backend_ast/shell/shell_word_expansion.inc, the changes are localized to the shellInvokeBuiltin function: (1) extend the initial if (!cmd || cmd->argc == 0) guard to also treat argv[0] == NULL or *argv[0] == '\0' as non-invocable, and remove the later if (!name || *name == '\0') block; (2) simplify the fprintf format arguments so they no longer test name ? name : "<null>", relying instead on the guarantee that name is non-null at that point.
| @@ -2506,16 +2506,13 @@ | ||
| } | ||
|
|
||
| static bool shellInvokeBuiltin(VM *vm, ShellCommand *cmd) { | ||
| if (!cmd || cmd->argc == 0) { | ||
| if (!cmd || cmd->argc == 0 || !cmd->argv[0] || *cmd->argv[0] == '\0') { | ||
| return false; | ||
| } | ||
| const char *name = cmd->argv[0]; | ||
| if (!cmd->ignore_functions && shellInvokeFunction(vm, cmd)) { | ||
| return true; | ||
| } | ||
| if (!name || *name == '\0') { | ||
| return false; | ||
| } | ||
| const char *canonical = shellBuiltinCanonicalName(name); | ||
| const char *lookup = canonical ? canonical : name; | ||
| if (shellRuntimeBuiltinDisabled(lookup)) { | ||
| @@ -2565,7 +2556,7 @@ | ||
| return false; | ||
| } | ||
| fprintf(stderr, "[builtin-missing] name=%s canonical=%s\n", | ||
| name ? name : "<null>", | ||
| name, | ||
| canonical ? canonical : "<null>"); | ||
| const char *label = canonical ? canonical : name; | ||
| if (vm) { |
| #if defined(PSCAL_TARGET_IOS) | ||
| ssize_t w = vprocWriteShim(STDOUT_FILENO, buf + off, len - off); | ||
| #else | ||
| ssize_t w = write(STDOUT_FILENO, buf + off, len - off); |
Check warning
Code scanning / CodeQL
Exposure of system data to an unauthorized control sphere Medium
| #if defined(PSCAL_TARGET_IOS) | ||
| ssize_t w = vprocWriteShim(STDOUT_FILENO, buf + off, len - off); | ||
| #else | ||
| ssize_t w = write(STDOUT_FILENO, buf + off, len - off); |
Check warning
Code scanning / CodeQL
Exposure of system data to an unauthorized control sphere Medium
Title: Merge devel into main.
Summary