[PATCH 2/2] sshbuf: forward the real error code from sshbuf_skip_string()

Muhammad Bilal meatuni001 at gmail.com
Fri Aug 28 18:16:10 AEST 2026


sshbuf_get_cstring() discards the specific error from its internal
sshbuf_skip_string(buf) call and returns a hardcoded -1
(SSH_ERR_INTERNAL_ERROR) instead:

    if ((r = sshbuf_skip_string(buf)) != 0)
        return -1;

every other error path in this function correctly forwards its own
'r'. This makes the skip_string path consistent with the rest of the
function.

Traced the reachability of this path: sshbuf_skip_string(buf) expands
to sshbuf_get_string_direct(buf, NULL, NULL), which re-peeks the same
buffer position sshbuf_get_cstring() already peeked successfully a
few lines above via sshbuf_peek_string_direct() (with nothing
mutating 'buf' in between), so that inner peek cannot itself fail
here. The only other failure path inside sshbuf_get_string_direct()
is an sshbuf_consume() error, which that function already collapses
to a hardcoded SSH_ERR_INTERNAL_ERROR (-1) regardless of the
underlying cause. So today, 'return -1' and 'return r' are
behaviourally identical for every input reachable through this call
site -- this is not currently observable misbehaviour. It is still
worth fixing: it matches the function's own established convention,
and it decouples this call site from an internal implementation
detail of sshbuf_get_string_direct() that could change in a future
refactor and start returning a more specific code (e.g. if consume's
own error were ever forwarded instead of collapsed), which would
silently go back to being masked here otherwise.

Compile-checked cleanly; no functional change to any currently
reachable code path.
---
 sshbuf-getput-basic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sshbuf-getput-basic.c b/sshbuf-getput-basic.c
index 77c0a782f..bf6db009b 100644
--- a/sshbuf-getput-basic.c
+++ b/sshbuf-getput-basic.c
@@ -285,7 +285,7 @@ sshbuf_get_cstring(struct sshbuf *buf, char **valp, size_t *lenp)
 		return SSH_ERR_INVALID_FORMAT;
 	}
 	if ((r = sshbuf_skip_string(buf)) != 0)
-		return -1;
+		return r;
 	if (valp != NULL) {
 		if ((*valp = malloc(len + 1)) == NULL) {
 			SSHBUF_DBG(("SSH_ERR_ALLOC_FAIL"));
-- 
2.55.0



More information about the openssh-unix-dev mailing list