bugfix(connection): Remove incorrect magic byte check for map transfers

[bobtista]

• Follow up for fix(connection): Implement file content validation for map transfers #1819
• Fixes Map transfer fails at 99% for certain players #2612

Removes the CkMp magic byte check at offset 0. Map files saved by WorldBuilder are RefPack-compressed and start with an EAR header, not the CkMp chunk tag. The CkMp tag only appears at offset 0 in uncompressed maps. The simplest fix is to remove this check, as the CkMp byte doesn't always end up at the same offset in compressed files. Also the other validation checks already handle the practical security issues - this check was more about rejecting malformed map files early.

[greptile-apps]

Greptile Summary

This PR removes the CkMp magic-byte check for map files in hasValidTransferFileContent, fixing a regression where WorldBuilder-compressed maps (which begin with an EAR/RefPack header, not CkMp) were always rejected during network transfer. The remaining guards — extension allowlist and 5 MB size cap — still apply, and the TransferFileType_Map case now performs no content-level check beyond those shared checks.

Confidence Score: 5/5

Safe to merge — the removed check was already broken for compressed maps; remaining extension and size guards still hold.

The only finding is a P2 style suggestion to add a dual-signature check (EAR or CkMp). The removal itself is correct: compressed WorldBuilder maps never satisfied the old check, so it was silently rejecting legitimate transfers. No P0 or P1 issues found.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp	Removes the broken CkMp magic-byte check for map files; the TransferFileType_Map case now has zero content validation (extension + size checks remain). Fix is correct but leaves an opportunity to add a proper EAR/RefPack or dual-signature check.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Incoming NetFileCommandMsg] --> B[hasValidTransferFileExtension]
    B -- invalid ext --> REJECT1[Drop / timeout]
    B -- valid ext --> C[Decompress TGA if needed]
    C --> D[hasValidTransferFileContent]
    D --> E{fileType?}
    E -- TransferFileType_Map --> F["Size ≤ 5 MB?"]
    F -- too large --> REJECT2[return false]
    F -- ok --> PASS1["✓ break — no content check after this PR"]
    E -- TransferFileType_Ini --> G["Null-byte scan"]
    G -- null byte found --> REJECT3[return false]
    G -- clean --> PASS2[✓ break]
    E -- TransferFileType_Tga --> H["Footer signature check"]
    H -- invalid --> REJECT4[return false]
    H -- valid --> PASS3[✓ break]
    E -- Other --> PASS4[✓ break]
    PASS1 --> WRITE[Write file to disk]
    PASS2 --> WRITE
    PASS3 --> WRITE
    PASS4 --> WRITE

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: Core/GameEngine/Source/GameNetwork/ConnectionManager.cpp
Line: 156-157

Comment:
**Consider replacing with a dual-signature check**

The `TransferFileType_Map` case now performs no content validation at all. Since the goal is to support both compressed maps (EAR/RefPack header, `0xFB 0x10` at offset 0) and uncompressed maps (`CkMp` at offset 0), a replacement check for either signature would restore early-rejection of garbage blobs while keeping the fix for compressed maps. Without it, any ≤ 5 MB file with a `.map` extension passes validation and gets written to disk, relying solely on the game's own parser to reject corrupt files.

```cpp
case TransferFileType_Map:
{
    // Accept either RefPack-compressed maps (EAR header 0xFB 0x10) or
    // uncompressed maps (CkMp chunk tag).
    const bool isCompressed   = dataSize >= 2 && data[0] == 0xFB && data[1] == 0x10;
    const bool isUncompressed = dataSize >= 4 && memcmp(data, "CkMp", 4) == 0;
    if (!isCompressed && !isUncompressed)
    {
        DEBUG_LOG(("Map file '%s' has unrecognized header.", filePath.str()));
        return false;
    }
    break;
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(connection): Remove incorrect magic ..." | Re-trigger Greptile}

[Mauller]

How about checking for either? Then unpacking and checking for the CkBk byte if you see the EAR magic byte?

[Skyaero42]

I agree removing it atm is the best approach. Insignificant impact on security and major blocker to play the patch.