fix: free scpBasePathDynamic before realloc in ParseScpCommand

A peer command that repeats the direction option (scp -t a -t b,
scp -f a -f b) re-enters the case and overwrites scpBasePathDynamic,
leaking the earlier buffer for the connection lifetime. Free and clear
the old buffer before each reallocation, and keep scpBasePathSz in
lockstep with the pointer the way wolfSSH_free() already does.

scpBasePath aliases that buffer and is only reassigned when the option
carries a path, so clear it alongside the free; otherwise a later
pathless -t/-f leaves it dangling into freed memory that
DoScpSource()/DoScpSink() hand to the scp callback.

Clearing the alias exposes a pre-existing case: a direction option with
no path parses to WS_SUCCESS with a NULL base path, which the callbacks
then dereference. Reject that after the parse loop instead.

Issue: F-6813
pull/1144/head
Mark Atwood 2026-07-23 11:39:04 -07:00 committed by Josh Holtrop
parent 82dd5814f8
commit 6cd440d7a6
1 changed files with 25 additions and 4 deletions

View File

@ -1540,13 +1540,21 @@ int ParseScpCommand(WOLFSSH* ssh)
case 't':
ssh->scpDirection = WOLFSSH_SCP_TO;
ssh->scpBasePathSz = cmdSz + WOLFSSH_MAX_FILENAME;
if (ssh->scpBasePathDynamic != NULL) {
WFREE(ssh->scpBasePathDynamic, ssh->ctx->heap,
DYNTYPE_BUFFER);
ssh->scpBasePathDynamic = NULL;
/* scpBasePath aliases the buffer just freed */
ssh->scpBasePath = NULL;
ssh->scpBasePathSz = 0;
}
ssh->scpBasePathDynamic = (char*)WMALLOC(
ssh->scpBasePathSz,
cmdSz + WOLFSSH_MAX_FILENAME,
ssh->ctx->heap, DYNTYPE_BUFFER);
if (ssh->scpBasePathDynamic == NULL) {
return WS_MEMORY_E;
}
ssh->scpBasePathSz = cmdSz + WOLFSSH_MAX_FILENAME;
WMEMSET(ssh->scpBasePathDynamic, 0, ssh->scpBasePathSz);
if (idx + 2 < cmdSz) {
/* skip space */
@ -1567,13 +1575,21 @@ int ParseScpCommand(WOLFSSH* ssh)
case 'f':
ssh->scpDirection = WOLFSSH_SCP_FROM;
ssh->scpBasePathSz = cmdSz + WOLFSSH_MAX_FILENAME;
if (ssh->scpBasePathDynamic != NULL) {
WFREE(ssh->scpBasePathDynamic, ssh->ctx->heap,
DYNTYPE_BUFFER);
ssh->scpBasePathDynamic = NULL;
/* scpBasePath aliases the buffer just freed */
ssh->scpBasePath = NULL;
ssh->scpBasePathSz = 0;
}
ssh->scpBasePathDynamic = (char*)WMALLOC(
ssh->scpBasePathSz,
cmdSz + WOLFSSH_MAX_FILENAME,
ssh->ctx->heap, DYNTYPE_BUFFER);
if (ssh->scpBasePathDynamic == NULL) {
return WS_MEMORY_E;
}
ssh->scpBasePathSz = cmdSz + WOLFSSH_MAX_FILENAME;
WMEMSET(ssh->scpBasePathDynamic, 0, ssh->scpBasePathSz);
if (idx + 2 < cmdSz) {
/* skip space */
@ -1596,6 +1612,11 @@ int ParseScpCommand(WOLFSSH* ssh)
ssh->scpDirection != WOLFSSH_SCP_FROM) {
ret = WS_SCP_CMD_E;
}
else if (ret == WS_SUCCESS && ssh->scpBasePath == NULL) {
/* direction set but no path parsed; don't hand a NULL base
* path to the scp callbacks */
ret = WS_SCP_CMD_E;
}
}
return ret;