Stop wPread and wPwrite dropping the high offset word

[yosuke-wolfssl]

Problem

wPread() / wPwrite() receive the SFTP file offset split into two 32-bit words, low word first — src/wolfsftp.c parses it that way and propagates carry into the high word. Two of the POSIX ports discarded that high word:

ret = (int)lseek(fd, shortOffset[0], SEEK_SET);   /* high word dropped */

• The lseek fallback, compiled when the platform has no pread/pwrite, seeked with the low word alone. An SFTP read or write at or past 4 GiB silently hit a masked position: wrong data returned to the client on read, corruption on write.
• The native pread/pwrite pair combined the high word only under SIZEOF_OFF_T == 8, so a target with a 32-bit off_t truncated the same way.
• Separately, (int)lseek(...) narrowed the returned position before comparing it against -1, so a valid seek to 0xFFFFFFFF was misread as an error and the transfer was skipped.

Fix (src/port.c, wolfssh/port.h)

New wResolveOffset() assembles the split offset in word64 and rejects anything above WOLFSSH_MAX_FILE_OFFSET — the widest value the seek call of the port can take, 2^63-1 for a 64-bit off_t and 2^31-1 otherwise. All four POSIX helpers reduce to that guard plus a cast, so the SIZEOF_OFF_T split disappears from each of them. Assembling in an unsigned type removes the signed-shift overflow, and the narrow ceiling also rejects a 2–4 GiB offset whose high word is zero. lseek is compared against (off_t)-1 before any narrowing. A rejected offset surfaces as WOLFSSH_FTP_FAILURE for that one request; the session stays up.

Scope: this covers the two POSIX ports. Harmony, Zephyr, Nucleus and the fseek fallback still seek with the low word alone, and FATFS's ff_pread/ff_pwrite take no offset argument at all. The helper is port-neutral, so each can adopt it by defining its own WOLFSSH_MAX_FILE_OFFSET. f-8823 stays open for those.

Tests (tests/unit.c, .github/workflows/os-check.yml)

test_PreadPwriteHighOffset() drives an offset of exactly 4 GiB: wPread() must report EOF past a 16-byte file — a truncating port rereads the start and returns data — and wPwrite() must leave st_size at 4 GiB + 1. The write half is skipped on a file system that fills holes, detected by a 1 MiB probe, so it never materialises 4 GiB; only EFBIG/ENOSPC may skip. The truncation lives in the lseek port, which no CI job compiled, so os-check gains an --enable-all CFLAGS=-DWOLFSSH_LOCAL_PREAD_PWRITE entry — the first CI coverage of that branch. The temp file now honours TMPDIR.

Verification

• make check on --enable-all and on the new forced-fallback config: 11 pass, 1 skip, 0 fail.
• Negative control: reducing the helper to the low word turns the test red on the fallback build (10 pass, 1 fail). Pointing TMPDIR at a missing directory fails the test, confirming the path comes from the environment.
• Helper contract checked standalone under ASan + UBSan against both ceilings: 4 GiB and 2^63-1 accepted, 2^63 and above rejected; narrow ceiling accepts 0x7FFFFFFF and rejects 0x80000000 and 3 GiB. No UB now that the assembly is unsigned.
• gcc-13 -Werror clean across 6 configs, including the Zephyr define set.

[Copilot]

Pull request overview

This PR fixes incorrect handling of SFTP/SCP file offsets in the wPread() / wPwrite() portability layer, ensuring the high 32-bit word of a split offset is not silently dropped and that 32-bit off_t builds fail closed instead of truncating. It also adds a unit test that detects the 4 GiB truncation behavior regression.

Changes:

• Fix offset assembly in src/port.c for both the lseek()-fallback and native pread()/pwrite() paths; fail when off_t is too narrow instead of truncating.
• Fix lseek() error detection by comparing against (off_t)-1 before any narrowing.
• Add a unit test covering a 4 GiB offset to catch truncation regressions.

Reviewed changes

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

File	Description
src/port.c	Correctly assembles 64-bit offsets (and fail-closed on narrow off_t) for wPread()/wPwrite() and fixes lseek() return handling.
tests/unit.c	Adds test_PreadPwriteHighOffset() to validate correct behavior at exactly 4 GiB offsets.

Suppressed comments (3)

src/port.c:187

• Same as in wPwrite(): assembling the 64-bit offset by left-shifting off_t can trigger undefined behavior for offsets >= 2^63. Build the combined offset in an unsigned 64-bit type and range-check before casting to off_t.

        #if SIZEOF_OFF_T == 8
            offset = ((off_t)shortOffset[1] << 32) | offset;
        #else
            /* off_t cannot hold the high word, fail rather than truncate */
            if (shortOffset[1] != 0)
                return -1;

src/port.c:208

• The 64-bit offset assembly uses ((off_t)shortOffset[1] << 32), which left-shifts a signed type and can be undefined behavior for offsets >= 2^63. Consider building in word64 and rejecting values that don’t fit in signed off_t before calling pwrite().

        #if SIZEOF_OFF_T == 8
            offset = ((off_t)shortOffset[1] << 32) | offset;
        #else
            /* off_t cannot hold the high word, fail rather than truncate */
            if (shortOffset[1] != 0)
                return -1;

src/port.c:224

• Same as in wPwrite(): building the offset with ((off_t)shortOffset[1] << 32) can be undefined behavior if the resulting signed off_t overflows (offsets >= 2^63). Build in word64 and range-check before casting to off_t for pread().

        #if SIZEOF_OFF_T == 8
            offset = ((off_t)shortOffset[1] << 32) | offset;
        #else
            /* off_t cannot hold the high word, fail rather than truncate */
            if (shortOffset[1] != 0)
                return -1;

---

💡 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 #1166

Scan targets checked: wolfssh-bugs, wolfssh-src

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

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

[ejohnstown]

The core fix is right, and catching the (int)lseek(...) narrowing as a second bug in the same helper is a good find -- a valid seek to 0xFFFFFFFF reading as an error is its own silent data-loss path.

I pulled the branch at 1fdb096 and reproduced your verification on macOS/APFS:

• --enable-all: make check 11 pass, 1 skip (external.test), 0 fail. PreadPwriteHighOffset ran the write half in full, so it really did materialise a 4 GiB + 1 sparse file and check st_size.
• --enable-all CFLAGS=-DWOLFSSH_LOCAL_PREAD_PWRITE: same result, no new compiler warnings.
• Negative control: reverting only the two lseek hunks and leaving everything else in place turns the test red. It does catch the bug it is written for, in the build that compiles that branch.

That last qualifier is the one thing I think should be settled before merge -- details inline on tests/unit.c. The other two inline notes are a correctness gap on narrow off_t and some duplication.

The other ports still drop the high word

Scope question rather than a defect in what you changed, and it spans several files so I could not pin it inline. This PR fixes the two POSIX ports, but the issue as titled -- wPread/wPwrite dropping the high offset word -- is still live in four others, all of which seek with shortOffset[0] alone:

• Harmony, src/port.c:129 and :142
• Zephyr, src/port.c:678 and :695
• Nucleus, wolfssh/port.h:824 and :838
• the fseek fallback, wolfssh/port.h:1183 and :1199

FATFS is worse: ff_pread/ff_pwrite (src/wolfsftp.c:2265, :2281) take no offset at all. Nucleus and the fseek fallback also only seek when ofst > 0, which assumes sequential access. Windows is fine -- RecvWrite/RecvRead set both OVERLAPPED.OffsetHigh and .Offset directly (src/wolfsftp.c:4410).

Fixing them all here would be a much larger change, so I would rather the PR body just said it covers the POSIX ports and that f-8823 stays open for the rest, instead of reading as if the class is closed. Harmony in particular is a one-liner away from consistency and was touched last week in 2a30f48.

[yosuke-wolfssl]

Hello @ejohnstown ,
Thank you for reviewing. I reworked on this.
I'll take other ports as follow-ups.