fix: ParseScpCommand leaks scpBasePathDynamic on repeated -t/-f

[MarkAtwood]

Bug

ParseScpCommand() (src/wolfscp.c) allocates ssh->scpBasePathDynamic in both
the -t and -f cases and assigns straight to the struct member without
freeing a previous allocation. The option scan does not consume the copied
remainder of the command, so a peer command that repeats the direction option
(scp -t a -t b, scp -f a -f b) re-enters the case, allocates again, and
overwrites the pointer, leaking the earlier buffer. The command string is
peer-controlled (wolfSSH_GetSessionCommand). The single teardown free
(internal.c) releases only the final pointer, so every prior allocation leaks
for the connection lifetime.

Fix

Free and clear ssh->scpBasePathDynamic before each (re)allocation.

Verification

Built --enable-all against wolfSSL master; compiles clean.

Reported by static analysis (Fenrir finding F-6813).

[Copilot]

Pull request overview

Fixes a memory leak in ParseScpCommand() (SCP command parsing) where repeated -t/-f options could overwrite ssh->scpBasePathDynamic without freeing the prior allocation, causing per-connection lifetime leaks on peer-controlled commands.

Changes:

• Free ssh->scpBasePathDynamic before re-allocating it in the -t parsing branch.
• Free ssh->scpBasePathDynamic before re-allocating it in the -f parsing branch.

Comments suppressed due to low confidence (2)

src/wolfscp.c:1526

• After freeing scpBasePathDynamic, ssh->scpBasePath can still point at the freed buffer. If the subsequent WMALLOC fails (or if the option has no trailing path and the later assignment isn’t hit), the session may retain a dangling scpBasePath pointer. Clear scpBasePath when freeing and set it to the newly allocated buffer immediately after allocation/memset so it always points at valid storage.

This issue also appears on line 1547 of the same file.

                        if (ssh->scpBasePathDynamic != NULL) {
                            WFREE(ssh->scpBasePathDynamic, ssh->ctx->heap,
                                    DYNTYPE_BUFFER);
                            ssh->scpBasePathDynamic = NULL;
                        }
                        ssh->scpBasePathDynamic = (char*)WMALLOC(
                                ssh->scpBasePathSz,
                                ssh->ctx->heap, DYNTYPE_BUFFER);
                        if (ssh->scpBasePathDynamic == NULL) {
                            return WS_MEMORY_E;
                        }
                        WMEMSET(ssh->scpBasePathDynamic, 0, ssh->scpBasePathSz);

src/wolfscp.c:1558

• Same as the -t case: after freeing scpBasePathDynamic, ssh->scpBasePath may still reference freed memory. Clearing scpBasePath on free and setting it to the newly allocated buffer avoids leaving a dangling pointer behind if allocation fails or if later code paths don’t reassign scpBasePath.

                        if (ssh->scpBasePathDynamic != NULL) {
                            WFREE(ssh->scpBasePathDynamic, ssh->ctx->heap,
                                    DYNTYPE_BUFFER);
                            ssh->scpBasePathDynamic = NULL;
                        }
                        ssh->scpBasePathDynamic = (char*)WMALLOC(
                                ssh->scpBasePathSz,
                                ssh->ctx->heap, DYNTYPE_BUFFER);
                        if (ssh->scpBasePathDynamic == NULL) {
                            return WS_MEMORY_E;
                        }
                        WMEMSET(ssh->scpBasePathDynamic, 0, ssh->scpBasePathSz);

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.