Monster spawning security has to be improved

[tlworks]

So I will try and get proof on this recorded on video but it's really hard to do so as there is a new hack going around (Still unknown to me) which allows the hacker to spawn certain amount of monsters, (most likely random ID's) I think there should be a permission to spawn in a monster without usage of a command.

[Ijwu]

Info for team members: @NyxStudios/tshock

As far as I know monster spawning is the only thing in Terraria which is actually, truly, server side. Unless this has changed in 1.3 I believe this issue may be caused by a client using an item or other method for spawning a monster that is not directly telling the server to spawn a monster.

@tlworks Thanks for the report, is it possible that you can get any more information? Can you possibly include some server logs which encompass a period in which this hack was used? Are you sure your permissions are properly set up to prevent players from using monster spawning items? Let us know any extra information you have.

[QuiCM]

Prior to 1.3, client-spawned mobs were only visible to the client that spawned them. This should still be the case post-1.3, as npc update packets (23) sent to the server are ignored.

[hakusaro]

There's basically no activity on this issue since June and no updates on it.

[Simon311]

Actually, I am unable to locate any TShock handler responsible for ReleaseNPC [71] packet. Therefore I assume it can be exploited to spawn NPCs.

[hakusaro]

Re-opening per @Simon311, tagging as security issue and PR wanted.

[hakusaro]

I am assigned to this issue for monitoring/oversight but am not actively working this issue.

[ivanbiljan]

professorx [1:57 PM]
@simon311 did you get a chance to explore https://github.com/NyxStudios/TShock/issues/1217 a bit more? GitHub Monster spawning security has to be improved · Issue #1217 · NyxStudios/TShock · GitHub So I will try and get proof on this recorded on video but it's really hard to do so as there is a new hack going around (Still unknown to me) which allows the hacker to spawn certain amount of mons...

simon311 [1:58 PM]
I do know we need to do something

[1:58]
But I don't entirely understand what

professorx [1:58 PM]
So an exploit is possible then?

nicatrontg [1:59 PM]
yes that’s what I thikn he means

[1:59]
maka what time is it in brisbaned

makaBOT [1:59 PM]
The time in Brisbane QLD, Australia is 9:59 PM 10/12/2016.

new messages simon311 [2:01 PM]
@professorx more like we need to eliminate a DoS attack vector and set our own limit of released NPCs

[2:01]
But no, I don't see how you would be able to spawn an NPC that is not officially marked as catchable

[ivanbiljan]

I believe the above should confirm this is not an issue. The "DoS attack" Simon was talking about comes from TSAPI's code and doesn't seem to be related to this.

[hakusaro]

@Simon311 could you comment?

[DogooFalchion]

Simple solution: is to rate limit the release of NPCs. Better solution: track NPCs caught by that player and only let them release what they have caught, and use some sane rate limit on both actions. Best solution: Use the DB to track the NPCs caught, in the event that the server goes down or restarts.

There are around 30 npcs that can be caught and spam released in the current TSAPI and TS. See Main.npcCatchable. No throttling currently exists.

[Simon311]

@DogooFalchion Neither of your solutions work. Rate limiting will only slow down the process of spamming, the second solution is bullshit because people can bring their own NPC items onto the server, the third solution is insane fucking overkill and has the same very obvious flaw as the second one. Also, Terraria Server only allows to release up to 200 * 0.75 / playercount NPCs, which is still a lot and needs to be regulated by us.

[hakusaro]

@Simon311 chiming in: I appreciate thinking about other issues, but please keep tone & language civil on github + the community.

@DogooFalchion I dunno if there's a good/better/best for this, since it's all kinda sloppy mitigation side for the last thing

[hakusaro]

What if we just say this is better suited to a plugin? I mean, all of this can happen outside of TShock right? And it's not super mission critical?

[DogooFalchion]

It certainly could be handled in a plugin, another question is do we add a hook for capture/release npc. I know those have packets but perhaps streamline it with a handler?

[hakusaro]

@DogooFalchion have no idea.

[ivanbiljan]

I'll just jump in quickly. I'll have to agree with Simon. Mainly because we'd be changing Terraria's behaviour, which again would get reported. I don't see the issue here; yes it loops over a bunch of stuff but it's not really that big of a deal since it always worked this way. Also, what you are talking about is far from the real issue, which is prooved to be non-existant. TLDR: close the issue but don't add any extra handlers.

[ivanbiljan]

@nicatronTg What is the point of this issue?

[ivanbiljan]

@hakusaro can we close this now?

[Ijwu]

Closed because this is old and nothing else will make @ProfessorXZ happier. Re-open if there is need.