Fix multiple reported issues

[kareem-wolfssl]

No description provided.

[Copilot]

Pull request overview

This PR hardens several parsing and buffer-handling paths across wolfSSH, addressing correctness and robustness issues in SCP parsing, path normalization, string utilities, KEX message parsing, certificate manager wrappers, and the SSH agent implementation.

Changes:

• Harden SCP parsing by replacing atoi() with strtoull() and adding stricter validation for file sizes and timestamps.
• Add/adjust bounds checks to prevent arithmetic underflow/overflow in path normalization, KEX parsing, and string concatenation helpers.
• Improve agent handling by avoiding accidental secret disclosure and fixing key identity comparison logic.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
src/wolfscp.c	Tightens SCP numeric parsing (file size + timestamps) with strtoull() and validation.
src/ssh.c	Strengthens wolfSSH_RealPath() bounds checking for segment copies.
src/port.c	Fixes wstrncat() free-space calculation to avoid size underflow scenarios.
src/internal.c	Adds bounds checks when skipping language name-lists during KEXINIT parsing.
src/certman.c	Initializes return value and adds basic NULL-guarding for root CA buffer loading.
src/agent.c	Avoids logging secrets by default, makes lock/unlock return failures on alloc, and fixes key-blob matching logic.

Comments suppressed due to low confidence (1)

src/ssh.c:3842

• The new bounds check doesn’t account for the optional '/' that may be appended when curSz != 1, and the WSTRNCAT() return values are ignored. This can still allow buffer-space miscalculation where the '/' append succeeds but the segment append fails (WSTRNCAT returns NULL), yet the function continues and returns WS_SUCCESS with a truncated path.

            if (segSz > outSz || curSz >= outSz - segSz) {
                return WS_INVALID_PATH_E;
            }

            if (curSz != 1) {

---

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

[lihnucs]

Hi Kareem,

I have reviewed all 7 commits in this PR and confirmed every fix is correct:

• 1c61fe4 (HIGH-01 / ssh.c): segSz > outSz short-circuit correctly prevents the unsigned underflow before the subtraction.
• 3ae9836 (HIGH-02 / agent.c): Size equality check id->keyBlobSz != keyBlobSz before WMEMCMP closes both the OOB read and the prefix-match auth bypass.
• 8ad669a (HIGH-03 / agent.c): All three defects addressed -- SHOW_SECRETS guard for the log, ForceZero before free, and exact-size heap allocation replacing the 32-byte truncating stack buffer.
• fd36f9a (HIGH-05 / wolfscp.c): strtoull + three-part validation (errno, endptr, > UINT32_MAX) correctly rejects -1 and all out-of-range values.
• 712fe71 (MED-11 / internal.c): skipSz <= len - begin guard on both language-list blocks prevents the begin wraparound.
• 9407efa (HIGH-04 / certman.c): NULL guard added before cm->cm dereference, matching the pattern in every other public CERTMAN API.
• 78390be (MED-01 / port.c): The strnlen-style bounded scan is more robust than a plain strlen -- handles the case where s1 already lacks a null terminator within n bytes.

All 131 CI checks are now passing. The fixes are well-constructed and I have no change requests.

One request before merge: could you please add a Reported-by trailer to each commit (or to the PR description) crediting the original reporter? The full preferred format is:

Reported-by: Asif Nadaf <postasif@protonmail.com>
This is the standard git trailer format used in most open-source security fixes and helps ensure the credit is preserved in the git log permanently. I notice a similar commit (989be64 in PR #1080) credited the reporter by name
only -- if that commit is amendable before merge, including the email there as well would be appreciated.

Thank you for the fast turnaround on all nine reports -- this was a thorough and well-executed response.