Skip to content

fix: correctness hardening across wolfSSH #1093

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
MarkAtwood wants to merge 1 commit into
base: master
Choose a base branch
from
+3 −3
Open

fix: correctness hardening across wolfSSH #1093

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/echoserver/echoserver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ static void *global_req(void *ctx)
return NULL;
}

wolfSSH_stream_read(threadCtx->ssh, buf, 0);
ret = wolfSSH_stream_read(threadCtx->ssh, buf, 0);

@aidangarske aidangarske Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

🟠 [High] Captured zero-length stream read now always shuts down global request sessions
🚫 BLOCK bug

The PR changes wolfSSH_stream_read(threadCtx->ssh, buf, 0) from a discarded call into the value tested by if (ret != WS_SUCCESS). That call cannot return WS_SUCCESS: wolfSSH_stream_read explicitly returns WS_BAD_ARGUMENT when bufSz == 0 in src/ssh.c:1230-1231. As a result, after every successful wolfSSH_global_request, the global-request thread prints wolfSSH_stream_read Failed., calls wolfSSH_shutdown(threadCtx->ssh), and exits. This is introduced by the PR because the previously stale ret still held the successful wolfSSH_global_request result, while the new assignment makes the invalid zero-length read control the error path.

Recommendation: Do not treat the zero-length wolfSSH_stream_read result as a successful keepalive/read result. Either remove this invalid read from the checked path, or replace it with a receive/worker path that is valid for processing the global-request reply and handles expected nonfatal statuses such as WS_CHAN_RXD, WS_WANT_READ, and WS_WANT_WRITE according to this thread's blocking mode. Add a WOLFSSL_TEST_GLOBAL_REQ regression path that verifies a successful global request does not immediately shut down the SSH session.

if (ret != WS_SUCCESS)
{
printf("wolfSSH_stream_read Failed.\n");
Comment on lines +322 to 325
Expand Down
2 changes: 1 addition & 1 deletion src/log.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
#endif


#ifndef WOLFSSL_NO_DEFAULT_LOGGING_CB
#ifndef WOLFSSH_NO_DEFAULT_LOGGING_CB
static void DefaultLoggingCb(enum wolfSSH_LogLevel level,
const char *const msgStr);
static wolfSSH_LoggingCb logFunction = DefaultLoggingCb;
Expand Down
2 changes: 1 addition & 1 deletion src/wolfsftp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9652,7 +9652,7 @@ int wolfSSH_SFTP_Get(WOLFSSH* ssh, char* from,
#elif defined(USE_WINDOWS_API)
{
DWORD desiredAccess = GENERIC_WRITE;
if (state->gOfst > 0)
if (state->gOfst[0] > 0 || state->gOfst[1] > 0)
desiredAccess |= FILE_APPEND_DATA;
state->fileHandle = WS_CreateFileA(to, desiredAccess,
(FILE_SHARE_DELETE | FILE_SHARE_READ |
Expand Down
Loading