OldUnreal / UnrealTournamentPatches

Other
976 stars 29 forks source link

[469b] UDP packets not sending correct data from udplink #876

Closed WinterSolstice8 closed 1 year ago

WinterSolstice8 commented 2 years ago

Versions of UT before 469b (did not test 469a) (as well as Unreal versions 227i and before) send a packet like this:

0010   0a 00 00 d1 0a 00 00 d1 0f 0a 0f 0a 00 15 d5 ba   ................
0020   82 52 65 71 75 65 73 74 3d 4f 66 66 82            .Request=Off.

(note the 0x82 in the data section)

UT469b, 227j, from windows:

0000   02 00 00 00 45 00 00 29 c9 2b 00 00 80 11 00 00   ....E..).+......
0010   0a 00 00 d1 0a 00 00 d1 0f 0a 0f 0a 00 15 5b bb   ..............[.
0020   3f 52 65 71 75 65 73 74 3d 4f 66 66 3f            ?Request=Off?

the 0x82 is now transformed to 0x3f

469b on linux resolves the wrong host (attempting to resolve 10.0.0.209 via .ini file, spits out Successfully resolved host 71.166.58.138:0). The cap also looks wrong from TCP dump (other caps are from Wireshark):

tcpdump -vv port 3850
tcpdump: listening on enp0s3, link-type EN10MB (Ethernet), capture size 262144 bytes
07:26:22.713642 IP (tos 0x0, ttl 64, id 44637, offset 0, flags [DF], proto UDP (17), length 44)
    unrealservers.lan.3851 > pool-71-166-58-138.bltmmd.fios.verizon.net.3850: [bad udp cksum 0x8d3c -> 0x527b!] UDP, length 16
tcpdump -xx port 3850
tcpdump: verbose output suppressed, use -v or -vv for full protocol decode
listening on enp0s3, link-type EN10MB (Ethernet), capture size 262144 bytes
07:35:08.262599 IP unrealservers.lan.3851 > pool-71-166-58-138.bltmmd.fios.verizon.net.3850: UDP, length 16
        0x0000:  b29a 520d fe31 0800 2760 53d9 0800 4500
        0x0010:  002c ef4e 4000 4011 be5f 0a00 00e3 47a6
        0x0020:  3a8a 0f0b 0f0a 0018 8d3c 4077 ec09 0000
        0x0030:  0000 6453 7973 7465 6d2f

so I can't get a "good" linux cap from 469b but here is 227j's cap:

0000   00 d8 61 76 50 7a 08 00 27 60 53 d9 08 00 45 00   ..avPz..'`S...E.
0010   00 2c 9b 6a 40 00 40 11 89 a3 0a 00 00 e3 0a 00   .,.j@.@.........
0020   00 d1 0f 0a 0f 0a 00 18 47 8a a0 68 e4 03 00 00   ........G..h....
0030   00 00 00 00 00 00 00 00 00 00 00 00               ............

It's data is nulled and has 2 extra bytes.

Repro steps:

1) Download ChatLink.zip and install to System folder 2) Modify chatlink.ini to have a ServerAddress IP to your LAN ip of choice to listen to packets 3) set up filter in wireshark for udp.port == 3850 or port 3850 with tcpdump 4) In a game mode that lets you summon any pawn (DM doesn't seem to work) summon ChatLink.ChatLink in game, or use ServerActors=ChatLink.ChatLink. You'll see some big output text in the log as follows if it spawned right:

//=====================================================
//       Inter-server communication protocol
//      by Pcube (www.freewebs.com/unrealpcube)
//              New stuff 2 enhance Unreal forever!
//
...

5) check packet, expect hex data of 82 52 65 71 75 65 73 74 3d 4f 66 66 82 in data section

I suspect this is caused from some ANSI conversion:

In ChatLink.ChatUDP: SendText( HostIP, querychar$"Request="$ServerName$querychar ); where querychar is set in ChatLink.ChatLink as CHR(130);

WinterSolstice8 commented 2 years ago

By request of Marco, I checked what happens on Linux if I prepend the string before the char(130), I edited the SendText call as follows: SendText( HostIP, "test"$querychar$"Request="$ServerName$querychar );

0000   00 d8 61 76 50 7a 08 00 27 60 53 d9 08 00 45 00   ..avPz..'`S...E.
0010   00 20 07 1d 40 00 40 11 1d fd 0a 00 00 e3 0a 00   . ..@.@.........
0020   00 d1 0f 0a 0f 0a 00 0c e4 34 74 65 73 74 00 00   .........4test..
0030   00 00 00 00 00 00 00 00 00 00 00 00               ............
an-eternity commented 2 years ago

Just a reminder here to check MODE_Binary and ReceivedBinary() in UdpLink as well (if they aren't still fixed in the latest v469c).

stijn-volckaert commented 2 years ago

TL;DR: 227j and 469c will add an IpDrv.InternetLink compatibility mode that restores the original (albeit incorrect) behavior for the execSendText and execReceiveText functions

Longer explanation: This is a nasty one. What is supposed to happen here is the following:

  1. Core.Object.Chr() embeds a 16-bit integer directly into an output FString without any sanity checks or conversions between character encodings
  2. SendText converts the first 1024 characters of its text argument from FString encoding to ANSI encoding and sends the resulting string to the host specified in the first argument

Now keep in mind that:

  1. 436/227i on Windows use UTF-16LE encoded FStrings. 436/227i (?) on Linux use ANSI-encoded FStrings. 469/227j use C++ wchar_t-encoded FStrings.
  2. Calling Core.Object.Chr(x) with x<=127 will result in the same (valid) FString on all of the above UT versions. Chr(x) with x=>128 is ambiguous. The raw data embedded into the output string will be the same, but the three UT versions will disagree on what the string represents.
  3. 436/227i's Linux FString to ANSI conversion was a no-op since FStrings were already ANSI-encoded. This is why Chr(130) worked reliably on those versions.
  4. 436/227i's Windows FString to ANSI conversion just discarded the top byte of every UTF-16LE character. This is not technically correct, but it worked reliably for your use case
  5. 469/227j's FString to ANSI conversion function correctly converts from your C++ wchar_t encoding to ANSI. The conversion from 0x82 (130) to ANSI depends on what your C++ wchar_t encoding is

To restore compatibility with ChatLink, we will make IpDrv's SendText and ReceiveText use conversion functions that emulate 436/227i's incorrect behavior.

Unrelated: Resolving 10.0.0.209 to an UnrealScript IpAddr should immediately succeed since 10.0.0.0/24 is a private IP range. It's possible that you do not see a "Resolved 10.0.0.209" message in the log for this very reason.

WinterSolstice8 commented 2 years ago

Yup, it's as I suspected. I did see that certain codepages allowed some >127 characters, but that could result in inconsistent behavior which we don't want.

Will check out the resolving, it looks like it's just wrong -- it's actually sending packets to the wrong IP as compared to what's in the ini file. Probably just needs a recompile, it's an ancient mod after all.

stijn-volckaert commented 2 years ago

I've pushed a fix that will be included in 469c-RC3

stijn-volckaert commented 2 years ago

The fix had some pretty nasty side effects so it had to be rolled back. I'll try to fix this another way for rc4

stijn-volckaert commented 1 year ago

I forgot to re-close this one. ChatLink should work as expected in the 469c release version (and in 469d). We added configurable text encoding support to IpDrv.InternetLink (and its subclasses). These are the two supported modes:

  45 │ //
  46 │ // OldUnreal: Text Encoding Mode.
  47 │ //
  48 │ // The InternetLink subclasses have several functions that send or receive ANSI text.
  49 │ // Since we store all strings in some Unicode format internally, all of these functions
  50 │ // require conversion between ANSI and the internal Unicode format.
  51 │ //
  52 │ // Unreal (<227j) and Unreal Tournament (<469c) performed this conversion by mapping
  53 │ // Unicode code points up to 0xFF directly onto ANSI characters with the same byte
  54 │ // representation, even if the characters represented by those bytes are not the
  55 │ // same as the original Unicode character.
  56 │ //
  57 │ // This behavior/feature is fine for Unicode code points up to 0x7F, since those code
  58 │ // points map directly onto (pretty much?) all ANSI code pages, but results in incorrect
  59 │ // translations for all points between 0x7F-0xFF.
  60 │ //
  61 │ // Unfortunately, there seem to be a few mods that rely on the old (incorrect) behavior.
  62 │ // To accomodate these mods, we use the old/incorrect encoding method
  63 │ // (TEXTENC_Compatibility) by default.
  64 │ //
  65 │ // New mods should, however, set TextEncoding to TEXTENC_Native. In this mode, the engine
  66 │ // converts unicode code points above 0x7F to UTF-8 characters.
  67 │ //
  68 │ var enum ETextEncoding
  69 │ {
  70 │     TEXTENC_Compatibility,
  71 │     TEXTENC_Native
  72 │ } TextEncoding;

The default value is TEXTENC_Compatibility, so this is what ChatLink will use in >469c.