Skip to content

Replace unsafe sprintf/_stprintf/wsprintf with safe equivalents

Historical reference only: stale-v0.72a-experimental-clean and analysis\stale-v0.72a-experimental-clean are retired reference sources, not active branch targets or current baselines. Use them only as provenance or idea-extraction sources; landed status is determined against main. See Historical References.

Summary

Approximately 40 occurrences of unsafe formatting functions (sprintf, _stprintf, wsprintf, _snprintf with no null-termination guarantee) across 11+ files. These are buffer-overflow risks, even when local buffers appear sufficient today. Replace with CString::Format, StringCchPrintf, or _stprintf_s.

Current Mainline Status

REF-023 is complete on current main. The cleanup landed in these app commits:

  • 1c4eeee (REF-023: remove MiniUPnP port sprintfs)
  • 8c5b509 (REF-023: remove debug endpoint sprintfs)
  • f1d2147 (REF-023: remove WebServer stprintfs)
  • dc37a5a (REF-023: remove remaining live sprintfs)
  • 1d992ca (REF-023: remove inert sprintf references)

The MiniUPnP slice:

  • CUPnPImplMiniLib::DeletePort() now formats mapping ports through FormatSafetySeams::FormatDecimalPortValueA() instead of a fixed char[8] plus sprintf.
  • CUPnPImplMiniLib::CStartDiscoveryThread::OpenPort() reuses the same helper for UPNP_GetSpecificPortMappingEntry() and UPNP_AddPortMapping().
  • Existing native coverage in format_safety.tests.cpp verifies decimal ASCII port formatting for 0, a normal eMule port, and 65535.

The debug endpoint slice:

  • DebugSend(), DebugSendF(), and DebugRecv() now format stored IPv4 endpoints through FormatSafetySeams::FormatStoredIPv4Endpoint() instead of fixed TCHAR[22] buffers plus _stprintf.
  • Two srand() reseed expressions in the same source file now use explicit unsigned int casts, removing the local C4244 warnings surfaced by recompiling OtherFunctions.cpp.
  • The local qsort comparator callbacks and sort function pointer contracts are marked noexcept, removing the local C5039 callback warnings surfaced by recompiling OtherFunctions.cpp.

The WebServer and SMTP slices:

  • WebServer progress, transfer, queue-score, and shared-file table formatting now use CString::Format instead of fixed buffers plus _stprintf.
  • WebServer transfer byte totals now accumulate as uint64, removing the local C5219 precision-loss warnings surfaced by recompiling WebServer.cpp.
  • The gzip header is emitted as fixed bytes instead of formatting byte values through sprintf.
  • The dead SMTP ciphersuite buffer initialization was removed; authentication paths overwrite the same buffer through mbedtls_base64_encode().
  • The final inert source comments were deleted or reworded so the source scan is zero-match for the unsafe formatting tokens.

Latest scoped validation for the closing slice:

  • python -m emule_workspace validate
  • python -m emule_workspace build app --config Debug --platform x64
  • Closing live-call slice completed all app targets with 0 warnings.
  • The inert-reference slice first rebuilt EmuleDlg.cpp and surfaced existing local warning debt; the immediate incremental pass completed all app targets with 0 warnings.
  • rg -n "\b(sprintf|_stprintf|wsprintf|_snprintf)\b" srchybrid --glob "*.cpp" --glob "*.h" returns no matches on main after 1d992ca.

Scope

Known files (from WWMOD audit, verify with grep): - srchybrid/EMSocket.cpp - srchybrid/ListenSocket.cpp - srchybrid/UploadQueue.cpp - srchybrid/DownloadQueue.cpp - srchybrid/SharedFileList.cpp - srchybrid/Preferences.cpp - srchybrid/KnownFile.cpp - srchybrid/PartFile.cpp - srchybrid/BaseClient.cpp - srchybrid/ServerConnect.cpp - srchybrid/EmuleDlg.cpp

Replacement Guide

Unsafe call Safe replacement
sprintf(buf, fmt, ...) sprintf_s(buf, _countof(buf), fmt, ...) or StringCchPrintfA(buf, _countof(buf), fmt, ...)
_stprintf(buf, fmt, ...) _stprintf_s(buf, _countof(buf), fmt, ...)
wsprintf(buf, fmt, ...) StringCchPrintf(buf, _countof(buf), fmt, ...)
_snprintf(buf, n, fmt, ...) (no null-term guarantee) _snprintf_s(buf, n, _TRUNCATE, fmt, ...)
Building a CString result CString str; str.Format(fmt, ...);

Prefer CString::Format when the result is stored in a CString anyway (avoids a fixed-size intermediate buffer entirely).

Relationship to REF-021

Removing _CRT_SECURE_NO_DEPRECATE (REF-021 WWMOD_007) will make the compiler emit deprecation warnings for all remaining sprintf/_stprintf sites. Running REF-021 first surfaces the full list automatically.

Recommended order: REF-021 → REF-023.

Files to Modify

Grep for \b(sprintf|_stprintf|wsprintf|_snprintf)\b (excluding _s variants) to get the exact list. Estimated ~40 call sites across 11 files.

Acceptance Criteria

  • [x] Zero occurrences of bare sprintf, _stprintf, wsprintf (non-_s variants) in srchybrid/
  • [x] _snprintf only used with _TRUNCATE flag
  • [x] No buffer sizes computed as sizeof(buf)-1 manual null-term hacks
  • [x] No remaining calls depend on _CRT_SECURE_NO_DEPRECATE; supported Debug x64 app builds pass after the replacements
  • [x] No functional behavior changes in string formatting output

Experimental Reference Implementation

Status in stale-v0.72a-experimental-clean: Essentially done. Unsafe call count dropped from 29 in main to ~0 in experimental (only one commented-out wsprintf and one reference inside a legacy Parser.cpp comment remain, both inert). The cleanup was distributed across REF-021 and REF-018 work — removing deprecated-Winsock and CRT warning suppressions forced fixing these sites. There is no single dedicated commit; the fixes are woven into the broader cleanup passes.

Porting note: After REF-021 removes _CRT_SECURE_NO_DEPRECATE, run a build and fix all newly-visible deprecation warnings. The fixes are mechanical one-line substitutions.