Skip to content

GetTickCount() 49-day overflow in ban expiry and download timeout checks

Implementation

Landed in eMule-main commit 6c161c0 (Migrate monotonic timing to GetTickCount64): - All GetTickCount() call sites across the codebase replaced with ::GetTickCount64() returning ULONGLONG - ClientList.cpp ban expiry and cleanup now use ULONGLONG curTick = ::GetTickCount64() - DownloadClient.cpp:1278 download timeout uses ::GetTickCount64() >= m_dwLastBlockReceived + DOWNLOADTIMEOUT - m_dwLastBanCleanUp, dwBantime storage widened to ULONGLONG - The 49-day overflow is eliminated; comparisons are now safe indefinitely

Follow-up hardening in eMule-main commit 56a312c changed the high-impact ban and download-timeout checks to elapsed subtraction form:

  • ::GetTickCount64() - dwBantime < CLIENTBANTIME
  • curTick - m_dwLastBanCleanUp >= BAN_CLEANUP_TIME
  • curTick - dwBantime >= CLIENTBANTIME
  • ::GetTickCount64() - m_dwLastBlockReceived >= thePrefs.GetDownloadTimeout()

Test commit 62ae9ef updates the tick-count regression helper to elapsed arithmetic and adds a near-UINT64_MAX wrap case.

Summary

GetTickCount() returns a DWORD (uint32) that wraps to zero after ~49.7 days of continuous uptime. Three call sites add a constant offset to a stored tick value and compare — when the sum wraps past DWORD_MAX, the comparison produces the wrong result: bans expire instantly or download timeouts fire immediately after the rollover.

Locations

ClientList.cpp:350 — Ban expiry check

#define CLIENTBANTIME HR2MS(2)  // 7,200,000 ms
return m_bannedList.Lookup(dwIP, dwBantime)
    && (::GetTickCount() < dwBantime + CLIENTBANTIME);

If a client is banned when GetTickCount() is near DWORD_MAX (e.g., 4,294,000,000), dwBantime + 7,200,000 wraps to ~902,032. The next call finds GetTickCount() = 4,200,000,000, which is NOT less than 902,032 → the ban appears to have expired immediately.

ClientList.cpp:455 — Ban cleanup (symmetric issue)

if (curTick >= dwBantime + CLIENTBANTIME)
    // remove from ban list

After wrap, dwBantime + CLIENTBANTIME is small; curTick is large → condition is true → ALL bans from before the rollover get cleaned up on the first cleanup pass after uptime exceeds 49.7 days.

DownloadClient.cpp:1278 — Download timeout

#define DOWNLOADTIMEOUT SEC2MS(100)  // 100,000 ms
if (::GetTickCount() >= m_dwLastBlockReceived + DOWNLOADTIMEOUT) {
    // disconnect / swap source
}

Same overflow: if m_dwLastBlockReceived is near DWORD_MAX, m_dwLastBlockReceived + 100000 wraps to ~99,705. GetTickCount() ≥ 99,705 is immediately true → download is aborted prematurely.

Root Cause

The safe pattern for DWORD tick arithmetic is:

// Safe — unsigned subtraction handles wrap correctly
if (::GetTickCount() - dwBantime >= CLIENTBANTIME)  // always correct

The unsafe pattern is:

// Unsafe — addition can wrap, breaking the comparison
if (::GetTickCount() < dwBantime + CLIENTBANTIME)   // wrong at 49-day mark

Fix

Replace the unsafe stored + constant addition pattern with now - stored >= constant:

// ClientList.cpp:350 — safe form:
return m_bannedList.Lookup(dwIP, dwBantime)
    && (::GetTickCount() - dwBantime < CLIENTBANTIME);

// ClientList.cpp:455 — safe form:
if (curTick - dwBantime >= CLIENTBANTIME)
    // remove from ban list

// DownloadClient.cpp:1278 — safe form:
if (::GetTickCount() - m_dwLastBlockReceived >= DOWNLOADTIMEOUT) {
    // disconnect / swap source
}

Unsigned subtraction wraps correctly in all cases and needs no special handling.

Scope

A grep for GetTickCount() >= .*+ and GetTickCount() < .*+ will surface all similar patterns across the codebase. The three sites above are the highest-impact ones (ban evasion, download abort), but the same issue exists in other time-comparison sites (see REF-011 for the broader GetTickCount→ std::chrono migration).

Acceptance Criteria

  • [x] ClientList.cpp:350 uses subtraction form: GetTickCount64() - dwBantime < CLIENTBANTIME
  • [x] ClientList.cpp:455 uses subtraction form: curTick - dwBantime >= CLIENTBANTIME
  • [x] DownloadClient.cpp:1278 uses subtraction form: GetTickCount64() - m_dwLastBlockReceived >= DOWNLOADTIMEOUT
  • [x] No new now < stored + constant DWORD tick comparison patterns introduced
  • [x] Bans and timeouts behave correctly across the 49.7-day DWORD rollover

Current Evidence

  • ClientList.cpp and DownloadClient.cpp now use elapsed subtraction at the tracked ban-expiry, ban-cleanup, cleanup-window, and download-timeout sites.
  • tickcount64.tests.cpp covers fixed-window ban/cleanup behavior past the legacy DWORD rollover and elapsed-window behavior when 64-bit deadline addition would wrap.
  • Latest validation:
  • python -m emule_workspace validate
  • python -m emule_workspace build app --config Debug --platform x64
  • python -m emule_workspace test all --config Debug --platform x64
  • Latest Debug x64 evidence: app build passed across main, CFG, community, broadband, and tracing-harness; native parity passed 490/490 and web API passed 64/64. Live-diff completed with expected main-only seam mismatch warnings.