Skip to content

IP filter update can delete the live `ipfilter.dat` before replacement promotion succeeds

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

Current main updates the IP filter through a destructive replace sequence that can leave the client with no ipfilter.dat at all.

PromoteIpFilterFile() deletes the existing live target first and only then tries to move the replacement into place. If the move fails, the old filter is already gone. The callers also ignore or compile away the helper's failure status and still mark the update as successful.

Landed In Main

Fixed in eMule-main by commit cc3553b (BUG-027 preserve ipfilter.dat on promotion failure).

The landed fix:

  • routes IP-filter promotion through the shared atomic replace helper instead of deleting the live target first
  • preserves the existing ipfilter.dat when replacement fails
  • sets bHaveNewFilterFile only when promotion actually succeeds
  • reports the real Win32 replacement failure to the user and trace output

Validated by supported app rebuild:

  • EMULE_WORKSPACE_ROOT\workspaces\v0.72a\state\build-logs\20260418-203331\summary.json

Evidence In Current Tree

  • srchybrid/PPgSecurity.cpp:57-66
  • PromoteIpFilterFile() does:
  • DeleteFileIfExists(strTargetPath)
  • then MoveFileEx(pszSourcePath, strTargetPath, MOVEFILE_REPLACE_EXISTING)
  • then returns PathExists(strTargetPath)
  • srchybrid/PPgSecurity.cpp:270-272
  • ZIP path ignores the return value and still sets bHaveNewFilterFile = true
  • srchybrid/PPgSecurity.cpp:300-302
  • RAR path does the same
  • srchybrid/PPgSecurity.cpp:334-336
  • GZip path does the same
  • srchybrid/PPgSecurity.cpp:367-369
  • plain-file path uses VERIFY(PromoteIpFilterFile(...)), which compiles away in release, then still sets bHaveNewFilterFile = true

Why This Is A Real Bug

MOVEFILE_REPLACE_EXISTING already supports replacement semantics. Deleting the target first creates an avoidable failure window.

If the replacement move fails because of a sharing violation, antivirus lock, temporary filesystem error, or any other runtime failure, the old ipfilter.dat has already been removed. The callers then proceed as if a new filter exists and reload the filter set.

That can silently downgrade the running client to an empty IP filter after an update attempt.

User-Visible / Runtime Impact

  • failed IP-filter updates can remove the previously working ipfilter.dat
  • the client can reload with zero IP filter entries after a failed promotion
  • release builds can treat a failed replacement as success because VERIFY(...) disappears

Cross-Variant Status

  • workspaces\v0.72a\app\eMule-main\srchybrid\PPgSecurity.cpp:57-66, 270-369
  • bug is present
  • analysis\stale-v0.72a-experimental-clean\srchybrid\PPgSecurity.cpp:253-372
  • bug is also present there
  • analysis\emuleai\srchybrid\PPgSecurity.cpp:268-386
  • bug is also present there

No audited sibling tree currently contains a clean replacement sequence for this path.

Likely Fix Shape

  1. stop deleting the existing target before replacement
  2. rely on an atomic/best-effort replace path (MoveFileEx(..., MOVEFILE_REPLACE_EXISTING), or a temp-file + replace helper) that preserves the old filter on failure
  3. make every caller check the returned success value before setting bHaveNewFilterFile = true
  4. report the real Win32 failure to the user/log instead of continuing as if the update succeeded

Validation Target

  • hold ipfilter.dat open or otherwise force replacement failure during update
  • verify the existing filter file remains intact
  • verify bHaveNewFilterFile stays false on promotion failure
  • verify OnReloadIPFilter() does not reload into an empty filter state after a failed promotion