Skip to content

fix: missing WMALLOC NULL checks in sftpclient rm/rename#1124

Open
MarkAtwood wants to merge 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/sftpclient-malloc-null-checks
Open

fix: missing WMALLOC NULL checks in sftpclient rm/rename#1124
MarkAtwood wants to merge 1 commit into
wolfSSL:masterfrom
MarkAtwood:fix/sftpclient-malloc-null-checks

Conversation

@MarkAtwood

Copy link
Copy Markdown
Contributor

Bug

In the sftp client example's doCmds(), the rm and rename handlers call
WMALLOC and then dereference the result without a NULL check:

  • rm: f = WMALLOC(...) then f[0] = '\0' (no check).
  • rename: the first buffer f is checked, but the second, fTo = WMALLOC(...),
    is dereferenced (fTo[0] = '\0') without one.

The allocations use the system heap (NULL ctx), so on allocation failure the
result is a real NULL and the write is a NULL dereference / crash. Sibling
handlers (e.g. mkdir, and rename's first buffer) already guard.

Fix

Add the same if (... == NULL) guard the sibling handlers use. For rename's
second allocation, free the already-allocated f before returning.

Verification

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

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

Copilot AI review requested due to automatic review settings July 23, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a NULL-dereference crash in the examples/sftpclient command handler by adding missing WMALLOC() failure checks in the rm and rename paths, consistent with other command handlers in doCmds().

Changes:

  • Add WMALLOC() NULL check for the rm handler before writing to the allocated buffer.
  • Add WMALLOC() NULL check for the rename destination buffer and free the already-allocated source buffer on failure.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants