Fix a one byte heap overflow in LoadTpmSshKey

[yosuke-wolfssl]

Problem

LoadTpmSshKey() builds an authorized_keys line by appending " <user>\n" and a terminating NUL to the contents of the key file, but allocates only length + usernameLen + 2 bytes — one short:

Write	Highest index touched
buffer[length] = ' '	length
WMEMCPY(buffer + length + 1, username, usernameLen)	length + usernameLen
buffer[length + 1 + usernameLen] = '\n'	length + usernameLen + 1
buffer[length + 2 + usernameLen] = '\0'	length + usernameLen + 2 — one past the end

The trim loop normally hides this. A key file ending in \n decrements length, which pulls every write back in bounds. A well-formed key file with no trailing newline leaves length untouched and the terminating NUL lands one byte past the allocation.

Fix (examples/echoserver/echoserver.c)

Reserve length + usernameLen + 3 so the separator space, the username, the newline and the NUL all fit. Identical one-line change in the copy at ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c.

Closes f-8820.

Tests (tests/api.c)

test_LoadTpmSshKey_NoTrailingNewline() stages a key file with no trailing newline, calls the loader and compares the assembled line. A second case with a \n-terminated file pins the trim path that already worked.

• LoadTpmSshKey() is no longer static and is declared in echoserver.h under WOLFSSH_TPM, matching how examples/client/common.h exposes ClientSetTpm().
• api.c now includes echoserver.h for TPM builds as well, not only the SCP/SFTP builds that set WOLFSSH_TEST_ECHOSERVER. Without this the test silently compiles out under --enable-tpm --enable-certs, where both are off.

Verification

• Negative control: ASan reports heap-buffer-overflow WRITE of size 1 at echoserver.c:2679, 0 bytes past a 71-byte region. Clean with the fix applied.
• Sanitizers: ASan + UBSan clean and api.test exits 0 under both --enable-tpm --enable-certs and --enable-all --enable-tpm.
• Compile sweep: GCC 13 -Werror, 6 configurations (enable-all, zephyr defines, sftp-only, scp-only, default, smallstack) all clean.

Not in this PR

No CI job compiles test binaries with --enable-tpm — tpm-ssh.yml runs make but never make check, and --enable-all does not include TPM — so this test does not yet gate merges. Tracked separately.

[Copilot]

Pull request overview

Fixes a one-byte heap overflow in LoadTpmSshKey() when building an authorized_keys-style line from a key file that does not end with a trailing newline, and adds an API test to prevent regressions.

Changes:

• Increase the WMALLOC() size in LoadTpmSshKey() from length + usernameLen + 2 to length + usernameLen + 3 so space + username + newline + NUL always fit.
• Expose LoadTpmSshKey() (non-static) under WOLFSSH_TPM via examples/echoserver/echoserver.h to enable direct testing.
• Add a TPM-focused API test that covers both the no-trailing-newline case and the trimmed trailing-newline case.

Reviewed changes

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

File	Description
tests/api.c	Includes echoserver header for TPM builds and adds a regression test for LoadTpmSshKey() newline handling.
examples/echoserver/echoserver.c	Makes LoadTpmSshKey() externally visible and fixes the allocation off-by-one.
examples/echoserver/echoserver.h	Declares LoadTpmSshKey() under WOLFSSH_TPM for test access.
ide/Espressif/ESP-IDF/examples/wolfssh_echoserver/main/echoserver.c	Applies the same one-byte allocation fix to the ESP-IDF copy.

Suppressed comments (1)

tests/api.c:1841

• Same as above: cleanup should assert remove(keyPath) succeeds to avoid leaving temp files behind and to match the pattern used elsewhere in this test file.

    remove(keyPath);

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1164

Scan targets checked: wolfssh-bugs, wolfssh-src

Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.