Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readd cooldowns on the world topic #48542

Closed
wants to merge 1 commit into from

Conversation

optimumtact
Copy link
Member

Thanks to whomever removed these, you're a real star

@optimumtact optimumtact added the Performance Uses the 32-bit address space and slow interpreter more effectively label Jan 2, 2020
@deathride58
Copy link
Contributor

putting the cooldowns/spam checks prior to TGS_TOPIC means that all of the relevant spam checks will essentially be bypassed, since most of the things that respond to topic calls are handled via TgsTopic()

@optimumtact
Copy link
Member Author

I actually mean to move that and forgot

@optimumtact
Copy link
Member Author

@Cyberboss @MrStonedOne

Thanks to whomever removed these, you're a real star
@MrStonedOne
Copy link
Member

This would trigger for the webserver, it requests topic at a rate of once per second.

var/static/list/topic_handlers = TopicHandlers()

//LEAVE THIS COOLDOWN HANDLING IN PLACE, OR SO HELP ME I WILL MAKE YOU SUFFER
if (addr in bannedsourceaddrs)
return
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use an associated list reference instead of in.

in is O(n), blah[key] is O(log n)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for some reason I thought that gives a runtime about the key if it doesn't exist

@NethIafin
Copy link

This would trigger for the webserver, it requests topic at a rate of once per second.

I think if you add TGS to some sort of IP whitelist...

@Cyberboss
Copy link
Member

Cyberboss commented Jan 4, 2020 via email

Comment on lines +183 to +188
if(world.time < (lasttime + 2 SECONDS))
log_admin_private("[addr] banned from topic calls for a round for too frequent messages")
bannedsourceaddrs[addr] = TOPIC_BANNED
return

lasttimeaddr[addr] = world.time
Copy link

@NethIafin NethIafin Jan 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead use
lasttimeaddr[addr] = world.time + 2 SECONDS
and check that
if(world.time < lasttime)

@MrStonedOne
Copy link
Member

I think if you add TGS to some sort of IP whitelist...

The webserver/game banners do not use TGS.

They do not come from localhost. they come from a lan ip for all servers except the eu servers, those see the internet ip of the webserver.

@MrStonedOne
Copy link
Member

This needs to be configurable, and should either be the normal standard format of count in time, or a better count in static timeframe system.

Look at the topic limiter for an example of a robust anti-spam system.

The ability to add ips on a whitelist would also help.

@NethIafin
Copy link

this doesn't appear to help. Tested close enough implementation on CM, stayed dead.

@Crossedfall
Copy link

Would still be good to have regardless. As for the whitelisting, could just open up a new config entry for that? That way you don't have to hardcode IPs into the handling.

@github-actions
Copy link
Contributor

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being closed by a maintainer if it is not updated or reviews are not addressed. If your PR is closed as stale, feel free to open a new one after dealing with the issues. This may also be an indication that the maintainers do not have interest in this change, you can try to convince them otherwise, or persist in the doomed world you have created.

@github-actions github-actions bot added the Stale Even the uncaring universe rejects you, why even go on label Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Uses the 32-bit address space and slow interpreter more effectively Stale Even the uncaring universe rejects you, why even go on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants