wiremod / advdupe2

Advanced Duplicator 2
http://wiremod.com
Apache License 2.0
90 stars 60 forks source link

validation for entity args #427

Closed Aws0mee closed 1 year ago

Aws0mee commented 1 year ago

Prior to this PR, clients could supply insufficient args when spawning an entity which will result in an error. https://github.com/wiremod/advdupe2/pull/417 mitigates this issue by deleting the bugged entities after they're spawned. However, this will still spam the server console with errors which can cause lag and be very annoying. This should properly fix the issue by preventing the bugged entities from being spawned in the first place.

thegrb93 commented 1 year ago

This breaks old dupes. Entity factory functions are supposed to be able to handle nil/invalid values.

Aws0mee commented 1 year ago

This breaks old dupes. Entity factory functions are supposed to be able to handle nil/invalid values.

Is there any way we can fix this? I don't like that somebody can just spawn a dupe with 5,000 broken entities to lag the server and spam my console. I already implemented this code on my own server, but I don't want to see the same thing abused on other servers.

thegrb93 commented 1 year ago

The erroring entity needs to be updated

Aws0mee commented 1 year ago

The erroring entity needs to be updated

what do you mean? It errors with default entities like buttons or lights

thegrb93 commented 1 year ago

Yeah those gotta be fixed. If its a player in particular abusing it, you could just ban them too.

Aws0mee commented 1 year ago

Yeah those gotta be fixed. If its a player in particular abusing it, you could just ban them too.

I already fixed the exploit and banned the players, but I wouldn't want the same exploit being abused on other servers. What do you think could be done for a proper fix? What old dupes would need to spawn entities with invalid args? Maybe we can whitelist entities that can spawn fine with invalid args, or blacklist entities we know will error with invalid args like buttons and lights.

thegrb93 commented 1 year ago

The facepunch/garrysmod repo accepts pull requests for fixes to lua code (sometimes).

For example, the wire light just got a new 'startOn' dupe param this week. Any dupe prior to this week without the new param would not be able to spawn a wire light if this PR got merged.

Aws0mee commented 1 year ago

The facepunch/garrysmod repo accepts pull requests for fixes to lua code (sometimes).

For example, the wire light just got a new 'startOn' dupe param this week. Any dupe prior to this week without the new param would not be able to spawn a wire light if this PR got merged.

So, the only way to properly fix this would be to edit each entity that has this problem and add defaults or something so the values are never nil?

thegrb93 commented 1 year ago

yeah