Server-side mods shouldn't be sent to clients

Ry0taK commented 3 years ago

Description

In MessageBuffer.cs.patch, there is a message type named ModFile: https://github.com/tModLoader/tModLoader/blob/dfe30aae2a3ecc8f5f7bc45db2132e386ddb90f5/patches/tModLoader/Terraria/MessageBuffer.cs.patch#L461-L466 When the server receives this message, the server sends back the mod contents with the name passed by the client. https://github.com/tModLoader/tModLoader/blob/dfe30aae2a3ecc8f5f7bc45db2132e386ddb90f5/patches/tModLoader/Terraria.ModLoader/ModNet.cs#L239-L260 However, since it doesn't verify that the mod specified here is a client-side mod, it allows the client to read server-side mods. https://github.com/tModLoader/tModLoader/blob/dfe30aae2a3ecc8f5f7bc45db2132e386ddb90f5/patches/tModLoader/Terraria.ModLoader/ModLoader.cs#L90-L94

Expected behavior

The server should only send back mods that are configured as a client-side mod.

Actual behavior

The server sends back mods that are configured as a server-side mod.

Additional Information

As it may reveal some sensitive information hard-coded in the server-side mod, I talked it with someone on the TML team. After discussing it, it seems it should be treated as a bug rather than a security issue. So I'm opening this issue.

Chicken-Bones commented 3 years ago

The client would need some way to know the mod name of the server mod, since this is not exposed anywhere to my knowledge. It's an easy patch regardless.

PragmaticBeaver commented 3 years ago

Hi everyone! I wanted to contribute and thought this issue would be a great starting point but I can't figure out how I manually test my fix because I can't find any Terraria Mods for tModLoader Version 0.12 (because it is still in development ;D).

So I guess my only option to evaluate my fix is to create my own micro mod? Could you kindly point me in the right direction?

cozykoko commented 3 years ago

https://discord.com/channels/103110554649894912/445276626352209920/791631101814767626 mods for 0.12 (Terraria 1.4)

If you dont want to bother with discord:

A list of mods available for 1.4 tML:

:x: - Not working. Please update with pull requests. :question: - Unknown. Please test and ping me with the results. :white_check_mark: - Up to date. Ping me if it breaks.

Also ping me if you ported a mod of yours.

Working on both 1.4 and 1.4_mergedtesting: :white_check_mark: MaxStackExtra https://github.com/kittenchilly/MaxStackExtra/tree/1.4 (fork) :x: Loadouts - https://github.com/xKirtle/Loadouts/tree/tModLoader-1.4.x :x: TerraTemp https://github.com/MutantWafflez/TerraTemp/tree/1.4-port

Targetting 1.4: :white_check_mark: HelpfulHotkeys https://github.com/kittenchilly/HelpfulHotkeys/tree/1.4 (fork) :x: CreativeTools https://github.com/NotLe0n/Creativetools/tree/1.4 :white_check_mark: ClickerClass - https://github.com/direwolf420/ClickerClass/tree/1.4_experiment (fork) :white_check_mark: CheatSheet - https://github.com/direwolf420/CheatSheet/tree/1.4 (fork) :x: HEROsMod - https://github.com/JavidPack/HEROsMod/tree/1.4 :x: Fargo's Mutant Mod - https://github.com/Fargowilta/Fargowiltas/tree/1.4

PragmaticBeaver commented 3 years ago

Thank you kindly, I will look into it!

PragmaticBeaver commented 3 years ago

I think, I still need some help. I can't find a way to call the aforementioned "ModNet.SendMod(reader.ReadString(), whoAmI); " method. From the code I thought, it would be called as soon as a client connects to the server but it doesn't look like it. The MessageBuffer class is called in a few places but it still isn't making sense to me. Could you kindly point me in the right direction again?

Chicken-Bones commented 3 years ago

Please come find us on Discord! Ping me or another dev and we'll give you access to #collaborators and help you out.

PragmaticBeaver commented 3 years ago

I think I got it but I just wanted to confirm the requirements because I'm not familiar with the mod side yet.

Inside the ModSide.cs there is an enum called ModSide which provides the following values:

Both
Client
Server
NoSync

After reading the comments I believe to fix this issue the server has to check for ModSide.Server on the mod object and simply do not send the data associated with it. Every other ModSide (Both, Client, NoSync) should be transferred. Is this correct?

ghost commented 2 years ago

Hi, is this do the thing? in MessageBuffer.cs at line 3536

                case MessageID.ModFile:
                    if (Main.netMode == 1)
                    {
                        ModNet.ReceiveMod(reader);
                    }
                    else
                    {
                        string modName = reader.ReadString();
                        Mod mod = ModLoader.GetMod(modName);
                        if (mod.Side == ModSide.Client)
                            return;
                        ModNet.SendMod(modName, whoAmI);
                    }
                    return;

I can also doing that but i dont know sendmod using in other functions or not. in ModNet.cs at line 241

        internal static void SendMod(string modName, int toClient) {
            Mod mod = ModLoader.GetMod(modName);
            if (mod.Side == ModSide.Client)
                    return;
            string path = mod.File.path;
            FileStream fs = File.OpenRead(path);

            {
                //send file length
                var p = new ModPacket(MessageID.ModFile);
                p.Write(mod.DisplayName);
                p.Write(fs.Length);
                p.Send(toClient);
            }

            byte[] buf = new byte[CHUNK_SIZE];
            int count;

            while ((count = fs.Read(buf, 0, buf.Length)) > 0) {
                var p = new ModPacket(MessageID.ModFile, CHUNK_SIZE + 3);
                p.Write(buf, 0, count);
                p.Send(toClient);
            }

            fs.Close();
        }

tModLoader / tModLoader