https://gitlab.synchro.net/main/sbbs/-/commit/795b48fcf3c4c6d6f7dbd2f5
Modified Files:
src/sbbs3/ftpsrvr.cpp
Log Message:
ftpsrvr: don't use a file record that loadfile() failed to fill
Three sites loaded a file record and used it without checking whether
the load had succeeded. Two of them left the record uninitialized, so a
failed load meant reading and then freeing indeterminate memory:
- the download credit check took the file's cost from an uninitialized
struct, giving an arbitrary verdict, and then called
smb_freefilemem() on its wild pointers
- the upload resume check left the same struct uninitialized along the
path that clears 'append' and falls through to smb_freefilemem()
- the delete uploader check zero-initialized the record but ignored the
result, leaving 'from' NULL for stricmp() to dereference
loadfile() re-opens the message base, so it can fail even though
findfile() has just succeeded: a lock timeout, or the record removed by
another session in between.
Each site now reports the failure and refuses the request. The record's
'from' pointer is checked before use as well, since
set_convenience_ptr() assigns it only when the header carries a SENDER
field: it can be NULL after a successful load, and the existing
"unknown user" fallback dereferenced it to test for an empty string.
The 'filedat' guards around two of those loads go with them. It is
assigned once and the request is already refused where it is false, so
the guarded fallback that charged by file length could never run, and
the guard's only remaining effect was to let an unloaded record reach
the checks.
Co-Authored-By: Claude Opus 5 (1M context) <
noreply@anthropic.com>
---
þ Synchronet þ Vertrauen þ Home of Synchronet þ [vert/cvs/bbs].synchro.net