fix: keep wc_RsaSSL_Sign()'s result signed in SignHashRsa

[yosuke-wolfssl]

Problem

SignHashRsa() stored wc_RsaSSL_Sign()'s int return directly into word32* sigSz, then tested if (*sigSz <= 0). Against an unsigned destination that test is equivalent to == 0, so every negative wolfCrypt error became a ~4 GiB length and fell through to the success branch.

An agent identity with an RSA key larger than 2048 bits makes wc_RsaSSL_Sign() return RSA_BUFFER_E, because PostSignRequest() passes a fixed 256-byte output buffer. wolfSSH_RsaVerify() then clamps the bogus length to the key size and copies that many bytes out of the 256-byte stack buffer — a 128-byte stack over-read at 3072 bits, 256 at 4096. The verify fails afterwards, so nothing reaches the wire; the defect is memory safety only.

Reachable through wolfSSH_AGENT_SignRequest(), which feeds whatever the agent I/O callback returns into DoMessage() — so a hostile or compromised agent socket can inject ADD_IDENTITY + SIGN_REQUEST. DoAddIdentity() applies no key-size limit. Gated on WOLFSSH_AGENT. Closes f-7203.

Fix (src/agent.c)

Keep the result signed and assign the length only on success, matching SignHRsa() in internal.c — the sibling call sites already did this correctly, and this was the only unsigned destination in the tree.

rc = wc_RsaSSL_Sign(encSig, encSigSz, sig, *sigSz, &key, rng);
if (rc <= 0) {
    ret = WS_RSA_E;
}
else {
    *sigSz = (word32)rc;
    ret = wolfSSH_RsaVerify(sig, *sigSz, encSig, encSigSz, &key, heap, ...);
}

Oversized keys now fail cleanly with WS_RSA_E.

Test harness (tests/api.c)

test_wolfSSH_agent_signrequest_oversize_rsa_key() reuses the existing WOLFSSH_AGENT harness, injecting two agent responses: an ADD_IDENTITY carrying a 3072-bit modulus, then a SIGN_REQUEST for it. AgentTestCtx.response grew 128 → 512 to carry them. Guarded by #ifndef WOLFSSH_NO_RSA_SHA2_256, matching the feature gates on the code it drives.

The fix changes no return code — WS_RSA_E before and after — so this test only distinguishes fixed from unfixed under a sanitizer.

Verification

• make check: 10 passed, 1 skipped, 0 failed.
• ASan + UBSan clean. Negative control with the fix reverted: stack-buffer-overflow, READ of size 384 at internal.c:13902 via wolfSSH_AGENT_SignRequest → wolfSSH_RsaVerify.
• Builds clean and passes with -DWOLFSSH_NO_RSA_SHA2_256 and -DWOLFSSH_NO_RSA; neutralizing the guard aborts in both, confirming it is load-bearing.
• GCC -Werror sweep: 6/6 configs clean.

[Copilot]

Pull request overview

Fixes a signed/unsigned conversion bug in the agent RSA signing path that could convert negative wolfCrypt errors into a huge signature length, leading to a stack over-read during RSA verification when using oversized RSA keys (under WOLFSSH_AGENT). Adds an API test that exercises the hostile-agent message injection scenario and is intended to be run under sanitizers.

Changes:

• Fix SignHashRsa() to keep wc_RsaSSL_Sign()’s return value signed and only assign *sigSz on success.
• Expand the agent test harness response buffer and add a sanitizer-focused regression test for an oversized RSA modulus injection path.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/agent.c	Prevents negative RSA sign errors from being stored into an unsigned size and erroneously treated as success.
tests/api.c	Adds a regression test (and increases harness response capacity) to detect the pre-fix stack over-read under sanitizers.

---

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1131

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅