The useAgent flag was never set, so both agent setup blocks in
wolfSSH_Client were dead code. Add the -a option to the parser and
carry it through the config struct, matching examples/client/client.c.
Issue: CID-572857
A failed WFSEEK() left the local file at offset 0 while the remote write
continued from the resume offset, silently corrupting the upload. Fail
out through the local close instead, the file is open at that point.
Issue: CID-572932
The return of SFTP_SetHeader() was ignored, unlike the other call sites.
Free the output buffer on the error path only when it was allocated here
rather than borrowed from the receive state.
Issue: CID-572923
The length guard allowed dirNameLen + 1 + dNameLen to equal the buffer
size, leaving no room for the terminating null, after which WSTRNCAT
appends nothing and the wrong path is used. Also terminate filePath
explicitly after the WSTRNCPY of dirName.
Issue: CID-572864
FindNextDirEntry() returns WS_BAD_ARGUMENT when its ctx is NULL, so by
the time control reaches the second check ret is already WS_BAD_ARGUMENT
whenever sendCtx is NULL. The earlier check right after the call is kept.
Issue: CID-572848
- add sftp_retry_remove()/sftp_retry_close() so Close and Remove are
driven to completion instead of abandoned mid-operation
- split the retry predicate: Open and SendWritePacket keep state only on
WS_WANT_READ/WS_WANT_WRITE, LS adds WS_REKEYING, Close and Remove gate
on NoticeError()
- assert the staging close; keep the post-read cleanup best effort, as
the reads are skipped rather than completed on WS_REKEYING
- route the WOLFSSH_TEST_INTERNAL write cleanup through the helpers
- shrink the staging chunk to 512 bytes
- Clamp the staging write chunk to the bytes remaining and derive the
try cap from the chunk count, so the loop lands exactly on
WOLFSSH_MAX_SFTP_RW for any value of the macro.
- Retry the staging Open and the LS on WS_WANT_READ, WS_WANT_WRITE,
and WS_REKEYING, matching the rekey tolerance used elsewhere in the
function.
- Assert the staging Open, the LS, and the listing match rather than
keying the read block on them, so neither a failed create nor a
connection stuck in WANT/REKEYING until the try cap can drop the
read coverage without failing the test.
- Remove any stale file before the staging Open, so the reads never
target a file staged by a prior aborted run.
- Size the read buffer from WOLFSSH_MAX_SFTP_RW, the amount staged and
the largest amount read, instead of the listed file size, and drop
the listed-size guard on the 18 byte read.
- Guard the listing name compare against a NULL fName.
- Verify every read returns the staged fill byte, not just a valid
length; the check is a macro so a failure reports the calling
read's line.
- Opening the listing's first entry raced with tests/testsuite.test,
which creates and removes files in the same directory under
"make -j check"; the entry could be gone before the open.
- Stage the read target over SFTP, select it from the listing by name,
and remove it afterward.
- Check the listing with AssertNotNull; a missing entry used to skip
the whole test body.
- Track the scpFileName allocation size in a new scpFileNameCap field,
so GetScpFileName() and ScpCheckForRename() reuse the buffer whenever
the name plus its terminator fits, instead of testing against the
previous name length
- Pass scpFileNameCap to the send callback, which writes into
scpFileName and needs the capacity; scpFileNameSz is now the name
length on every path, including the source path
- Free and reallocate the transfer buffer in ScpSourceInit(), clearing
the size fields alongside the pointers they describe
- Wrap a long line in ScpProcessEntry()
- Add test_ScpGetFileName covering the reuse-vs-realloc branch, the
exact-fit boundary, a grow-by-one that catches an off-by-one in the
reuse condition, and a source-path buffer holding no name yet
- Add test_wolfSSH_SCP_RecursiveTwoFiles, a real "scp -r" transfer of
two files, covering the scpFileHeaderSent reset on the recursive
path added in the duplicate-header fix; it clears leftovers from an
aborted run up front and bounds its recv so a regression fails
instead of hanging. Not built on Windows, where a recursive transfer
through the default callbacks does not reproduce the sent file
- Gate header on new scpFileHeaderSent flag, not scpFileOffset==0
- A send callback returning 0 bytes first no longer re-sends header
- Skip zero-length SCP_SEND_FILE send to avoid empty CHANNEL_DATA
- Abort on a second consecutive 0-byte send callback return with file
data outstanding; skipping the send would otherwise spin
SCP_SEND_FILE -> SCP_TRANSFER with no socket I/O
- Reset offset/bufferedSz/flags in ScpSourceInit for connection reuse
- Document the WS_CallbackScpSend contract, including when a 0 return
is valid, next to the typedef
- Add test_wolfSSH_SCP_SendZeroFirst regression (func_args scp_send hook)
- Zero func_args in kex.c/testsuite.c so the new scp_send field is not read uninitialized
Issue: ZD-22176
SHELL_Subsystem() raises privileges to look up user information, but the
three pipe() failures and the forkpty() failure return WS_FATAL_ERROR
without dropping them again. HandleConnection() then runs its teardown --
wolfSSH_shutdown(), up to ten wolfSSH_worker() iterations, and the socket
drain -- still elevated when UsePrivilegeSeparation is yes or sandbox.
Drop permissions before each of the four returns, reusing the wording the
daemon already logs for a failed drop.
The forkpty() path also returned with all six pipe descriptors still
open. Close them there, as the pipe() failure paths already do, and reset
the slots to -1 to keep the child-branch invariant that an entry is
either a live descriptor or -1.
Issue: F-6980
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
Raise a cryptographic minimum to current guidance. It rejects peers below
the new floor by design.
- Reject RSA user-authentication keys smaller than 2048 bits with
WS_CERT_KEY_SIZE_E, per NIST SP 800-131A. The floor is configurable via
a new WOLFSSH_RSA_MIN_KEY_BITS define (default 2048).
Issue: F-6517
The send at the end of the loop only ran when select() reported a
readable descriptor, but appBuffer can hold data across iterations.
Mask appFd out of the read set while the buffer is full, and fall
through the select() timeout to the send.
Issue: F-7207
- AeadIncrementExpIv() returned as soon as a byte incremented without
wrapping, so the iteration count depended on the counter, which starts
out as KDF output. Carry through all 8 bytes instead.
- Same counter values and the same instruction count once unrolled; the
built object no longer has the data-dependent exits.
Issue: F-7216
- ret held the read length from wolfSSH_SFTP_buffer_read(), and was only
reset at the bottom of the per-entry loop, so a NAME response with a
count of 0 skipped the loop and logged a read error.
- Set ret to WS_SUCCESS after the count is parsed. The NULL return and
untouched ssh->error are unchanged; only the bogus log goes away.
Issue: F-7215
- wolfSSH_AGENT_worker() dereferenced ssh->agent to set the DONE state
without checking it, so a call before the agent was set up took a null
dereference.
- Return WS_AGENT_NULL_E instead.
Issue: F-7214
- wolfSSH_MakeRsaKey and wolfSSH_MakeEcdsaKey called wc_FreeRsaKey and
wc_ecc_free unconditionally, so a failed init left the free operating
on an indeterminate stack struct.
- Track init with a keyInit flag, as wolfSSH_MakeMlDsaKey does, and free
only when the init succeeded.
- Guard wc_ed25519_free in wolfSSH_MakeEd25519Key the same way.
- Replace the tab indent on the ed25519 free with spaces.
Issue: F-7212
- The zero check ran WSTRCMP() on the caller's buffer, which is a
length-bounded slice of the config line and not NUL terminated, so it
read past inSz and rejected valid "0" values whose slice had trailing
text.
- Compare num, the NUL-terminated copy that atol() was given.
Issue: F-7213
- crypt() returns a pointer into a static buffer that keeps the hashed
password after CheckPasswordHashUnix() returns; wipe it once the
comparison is done.
Issue: F-7220
- Track userPublicKey allocations with a userPublicKeyAlloc flag
- Free on that flag, not pubKeyName; -J with no -j leaked the cert
- Restore userPublicKeyBuf when a key load fails, not a stale pointer
- Tag load_der_file() allocations DYNTYPE_PRIVKEY to match the frees
- Free the CA cert in ClientLoadCA() with the heap it came from
- Pass the caller's heap into wolfSSH_TPM_InitKey() so the TPM public
key is allocated from the pool ClientFreeBuffers() frees it with
Issue: F-7210, F-7211
- IdentifyAsn1Key() was called with a literal 1, so a public PEM decoded
by wc_PubKeyPemToDer() was run through the private-key decoders and
never identified.
- Add test_wolfSSH_ReadPublicKey_pem() covering a public RSA PEM read
through wolfSSH_ReadPublicKey_buffer(). The read itself only compiles
with WOLFSSH_TPM, so the test also asserts the isPrivate 0 vs 1
difference in IdentifyAsn1Key() directly, which every build runs.
Issue: F-7209
- Zero-initialize struct tm t; it was read indeterminate when the parse
failed.
- Fail the profile when wc_GetDateAsCalendarTime() returns non-zero, and
gate the two date-format comparisons on valid.
Issue: F-7208
- Test wolfSSH_ReadKey_buffer() and wc_CertPemToDer() results and skip
the entry; a malformed file gave PwMapNew() a NULL buf and an
indeterminate length.
- NULL-check both WMALLOC() results, and guard the load_file() size so
a missing file no longer reaches WMALLOC(0).
- Fall through to the loop's existing WFREE()/advance so both buffers
are freed on every path.
Issue: F-7204
c[inSz-1] = 0 clobbered the last byte of the copied key data instead
of terminating the string after it, truncating public keys by one
character.
Issue: F-7205
- wolfSSH_ChannelSend() can return fewer bytes than requested, but
appBufferUsed was decremented without moving the unsent tail to the
front of appBuffer, corrupting subsequent sends. Shift the remaining
data down with WMEMMOVE() when the send is short.
- Skip the recv() when appBuffer is full. The length argument would be
0, and a zero length recv() returns 0, which the loop read as the
peer closing; the buffered data was dropped and the forward torn
down.
Issue: F-7207
heap was used throughout certman.c (WMALLOC/WFREE calls, and the
free at wolfSSH_CERTMAN_free) but was never stored on the struct
during _CertMan_init, so it was always NULL.
Issue: F-7206
kSz can change (e.g. after rekeying), so it may no longer cover the
full extent of previously stored key material. Use sizeof(ssh->k)
instead to ensure the whole buffer is wiped, and reset kSz to 0.
Issue: F-7219
Directory handles are tracked on ssh->dirList, a separate list from the
file handles, so the SFTP_AddFileHandle cap does not reach them. A peer
could loop on OPENDIR and grow that list until allocation failed.
- Add SFTP_DirHandleCapped() and check it in both RecvOpenDir branches
before the directory is opened or the name buffer allocated, so the
rejection has nothing to unwind. Refusal sends an SFTP status like the
existing permission-denied path rather than dropping the request.
- Reuse WOLFSSH_MAX_SFTP_HANDLES, so each list is bounded separately.
- Add wolfSSH_SFTP_TestDirHandleCount() and a regress test.
An authenticated peer could open SFTP file handles without limit,
growing ssh->fileList until allocation failed and making the linear
handle lookup progressively more expensive.
- Limit tracked file handles per session to WOLFSSH_MAX_SFTP_HANDLES
(default 64), bounding memory and the linear-lookup DoS surface.
- Check the cap in SFTP_FileHandleCapped() from both RecvOpen branches
after the path is resolved and before the file is opened. The open
flags derived from the request may carry O_CREAT and O_TRUNC, so
refusing after the open would create or truncate the target on a
request the peer is told failed. SFTP_AddFileHandle() keeps the check
as a backstop.
- Refuse with FTP_FAILURE "Too Many Open File Handles" so the peer can
tell a resource limit from a server malfunction.
- Add a regress test via wolfSSH_SFTP_TestFileHandleCount() covering the
cap, the refusal status reply, and that a refused open leaves the
target file untouched.
- Scenarios 1 and 5 fopen() once and fstat() that handle instead of
stat()ing the path a second time.
- A failed open is now a logged error rather than an indirect rd == 0.
- The FIFO lstat() and failPath existence test stay as they are: neither
has a descriptor to stat, and mkdtemp()'s 0700 directory makes them
unraceable.
Issues: CID-651701
- noneOk was only ever set inside the WOLFSSH_ALLOW_NONE_CIPHER guard,
so usableCount++ was unreachable in the default build.
- Move the whole ID_NONE arm under the guard and drop noneOk. Behavior
is unchanged in both builds.
Issues: CID-651702
- GetNameListRaw stops at the first zero-length name and drops the
rest, matching OpenSSH's match_list() on a peer proposal.
- Its comma test lagged the index by one, folding a doubled comma into
the preceding name.
- An empty name-list is a list of zero names, not WS_BAD_ARGUMENT.
- Drop CountNameList. DoExtInfoServerSigAlgs parses into an array
capped at WOLFSSH_MAX_NAMELIST_CNT and allocates only on a match. A
list naming nothing it can sign with is advisory, RFC 8308 section
3.1, so it clears any recorded list instead of failing the
connection. The latest server-sig-algs wins, as in OpenSSH.
- Guard the list[0] reads in DoKexInit on a non-empty list. No change
today, an empty list fails to match first.
- Add wolfSSH_TestDoExtInfo and tests for both name-list paths, the
caps, lists of more than one algorithm, and a second EXT_INFO
superseding the recorded list, empty and unusable values included.
Issue: F-6817
Add mutation-killing unit coverage flagged by Fenrir static analysis.
No production logic changes.
* F-6702 test_configuration.c: extend test_CheckPasswordHashUnix with
empty/locked-password cases pinning the empty-branch guard.
Co-authored-by: John Safranek <john@wolfssl.com>
- src/internal.c DoKexDhReply: drop the stale fuzz note on the signature
size. The size checks it pointed at stay.
- tests/unit.c: replace an em-dash with a comma, for ASCII-only sources.
None of the three paths this branch changed had coverage, which is why
make check passed both before and after the SendChannelData regression.
- unit: SendChannelData and SendChannelExtendedData with a peer maximum
packet size of 0 report WS_WINDOW_FULL, queue nothing, and leave
peerWindowSz alone. A zero-length send still succeeds.
- unit: BuildNameList terminates buf for an empty id list. The buffer is
poisoned first, so a missing terminator shows up as a wrong length
instead of depending on what the allocator handed back. Reaches the
static function through a new wolfSSH_TestBuildNameList hook.
- regress: a KEXDH_REPLY whose signature blob holds nothing but a name
length prefix is rejected with WS_BUFFER_E, pinning the bounded read
that replaced the hand-rolled name parse.
Each test was confirmed to fail with its fix reverted.
Issue: F-4586, F-4587, F-6525
DoMessage hand-rolled the payload length check: a five-byte room test,
then ato32, then payloadSz > len - begin. GetSize does both bounds in one
call, and a nonzero payloadSz already covers the message id byte that the
MSG_ID_SZ term reserved. The failure still reports WS_OVERFLOW_E rather
than the helper's WS_BUFFER_E, so the agent's error codes are unchanged.
Issue: F-6693
Address a batch of integer-underflow and bounds findings from Fenrir
static analysis.
- src/internal.c ChannelNew: skip channel ids already in use so a wrapped
word32 nextChannel cannot collide with a live channel.
- src/internal.c GetNameList: drop the length pre-check. GetStringRef
bounds both the length prefix and the list, and the pre-check also
rejected a valid empty name list ending exactly at len.
- src/internal.c DoKexDhReply: parse the signature name and blob with
GetStringRef and GetSize instead of GetUint32 plus hand-rolled
remainder checks, which underflowed sigSz - begin - LENGTH_SZ for a
sigSz of 4 to 7 and let a name comparison read past the packet. Retires
the redundant sigSz + begin + tmpIdx > len check in the RSA path.
- src/internal.c BuildNameList: return 0 for srcSz == 0, before any *src
deref or srcSz-- underflow, terminating buf for the callers that
measure it with WSTRLEN.
- src/internal.c SendChannelData/SendChannelExtendedData: report
WS_WINDOW_FULL when the computed bound is zero, rather than sending a
zero-length packet.
- src/wolfsftp.c wolfSSH_SFTP_DoStatus: clamp out-of-range status to
WOLFSSH_FTP_FAILURE so it cannot alias a negative WS_* error code.
A malformed KEXDH signature length prefix now reports WS_BUFFER_E from
the parse helper rather than WS_PARSE_E. The signature name mismatch
still reports WS_PARSE_E.
Issue: F-2480, F-3449, F-4586, F-4587, F-6525, F-6697
John Safranek
Yosuke Shimizu
JacobBarthelmeh
Mark Atwood
Emma Stensland