Skip to content

Conversation

PhrozenByte
Copy link
Contributor

The /report GUI doesn't disable keybinds, so (for example) typing a t into one of the input fields fires the chat bind and steals input focus. If you close the /report GUI while the admin panel is visible in the background, the mouse disappears and leaves a unusable admin panel. After successfully submitting a message, a small window (aMessageBox) is shown. The window also shows an Ok-button, which isn't reachable, because the mouse is already hidden. This misleads users to press Enter, what is often a bound key (e.g. /kill in the race gamemod), even though the window is automatically hidden after 3 seconds.

Steps To Reproduce

  1. load & start admin resource
  2. login as admin
  3. open admin panel (default: p)
  4. type /report
  5. focus is stolen when typing a bound key (e.g. t) into one of the input fields (addressed by the first commit)
    6.1. when closing the /report window, the mouse disappears while the admin panel is still visible (addressed by the second commit)
    6.2. when submitting, a success message is shown as described above (addressed by the third commit)

Mantis
First commit: http://bugs.mtasa.com/view.php?id=8791
Second commit: -
Third commit: -

@PhrozenByte PhrozenByte changed the title [admin] Fixing keybinds/mouse visibility of /report [admin] Fixing /report Mar 18, 2015
@qaisjp qaisjp added the bug label Sep 10, 2015
@PhrozenByte
Copy link
Contributor Author

More than a year later neither merged nor rejected or at least commented... 😞

@qaisjp qaisjp self-assigned this Apr 2, 2016
@PhrozenByte PhrozenByte deleted the bugfix/admin_report branch June 29, 2016 22:54
@PhrozenByte PhrozenByte restored the bugfix/admin_report branch June 29, 2016 22:56
@PhrozenByte PhrozenByte deleted the bugfix/admin_report branch June 29, 2016 22:56
jushar pushed a commit that referenced this pull request Jan 14, 2017
…se lag (#60)

* Block a type of server (lag) flooding in freeroam setskin

I recently got to meet a way for players to mass-lag the server, putting heavy load on it through setting skin in F1 set skin GUI repeatedly hitting 'set' button in teamwork with a few other players. Since the function is expensive and serverside, spamming the button with some other players can cause significant lagspikes on the victim server.

This patch fixes the vulnerability because the effectiveness of that attack type relies on firing the setskin function very rapidly, this patch prevents the unnecesary processing of the set skin when the same skin they are applying is already the current skin.
It works because achieving high 'set' button press rates doesn't work if they got to select another skin time over time, this makes them lose seconds that make or break the lag effect.

* Second patch of lag-inducing attack method in Freeroam

Addition to previous commit, also prevent setskin command method of same server lagging method

* Addendum to fit coding guidelines

Following code review/arran's comments

* Improvement: detect deliberate spam/macro on command and block

This change also temporarily (10s) ignores requests from a client deliberately spamming the command.
Legitimate use f.e an player wanting to do something faster, doesn't reach the speeds of triggering command this detection goes off on.

Compare when I can safely hit at rate botched by only the 2 seconds timer (first part of video): https://www.youtube.com/watch?v=ZuCyICDeOhg
versus near video end the (almost inhumane) speed I start hitting the bind like a madman, which is where the 10 sec command ignore kicks in.

So, this improvement further protects against deliberate spammers by fencing off even more than the 2 seconds limit (they would be able to induce lag every 2 seconds otherwise when with much more players doing it even if limited to 2s)
without impacting gameplay of legitimate players just looking to speed something up.

* Added protection to final server-call functions that are vulnerable to same type of attack

Enabled the timer and deliberate spam detector to the remaining server-calling functions aswell capable of inducing lag/load in the same manner as setskin function.

* Move settings to meta.xml

Per the codereview recommendations

* Move settings to meta.xml #2

* Improvements

Changed .lua settings to meta and also several logic improvements, most of all terminating any interference with gameplay experience even further than asked in codereview by removing the once-per-2 seconds limitation for commands, and instead only detecting many commands fired *within* 2 seconds, more accurately detecting intended spam but only triggering the suspend of calls from spamming client if it's fired at excessive rates (the default value in meta is 5, pressing a bind/command 5 times within 2 seconds is unusual)

Also added the same protection mechanism for the give weapons/ammo GUI, as it can be abused in a similar way and the serverside function is also pretty expensive, capable to cause lag if spammed excessively. Will by default also trigger when hit 5 times within 2 seconds.

* tweak code

* fix typos
Dutchman101 pushed a commit that referenced this pull request Sep 4, 2017
Dutchman101 pushed a commit that referenced this pull request Sep 21, 2017
This is the second PR containing commits aiming to improve Freeroam codebase quality with improvements & cleanup, and performance/effiency optimizations.
Together with previous PR (#78), memory and resources usage should be better.

* Added exception commands setting for antispam protection
* Improvements to vehicle spawning, including fix for space-containing vehicle names (like now, /cv Super GT works)
and some additional fixes, clear from the diff
Dutchman101 pushed a commit that referenced this pull request Mar 1, 2018
After the codereview recommendations, changed "US" into N/A if it fails to determine the flag.
Besides that, now the country column wont be blank when admin resource misses flags (still the case with some countries) but draw the countrytag regardless. I will work on adding some commonly missing flags to the admin resource soon.
Dutchman101 pushed a commit that referenced this pull request Dec 31, 2019
jlillis pushed a commit that referenced this pull request Nov 10, 2020
* votemanager: store empty string for vote reason instead of nil if none is provided

Saving nil in a table seems to have odd side effects when further adding key values to it. 

local tab = {true,false,false,nil,5} tab.cheese = "Yes" local a,b,c,d,e = unpack (tab) outputChatBox(tostring(e))

Should output 5 but instead outputs nil. If I remove the tab.cheese = "Yes"

local tab = {true,false,false,nil,5} local a,b,c,d,e = unpack (tab) outputChatBox(tostring(e))

It will correctly output 5 again. However I don't see any easy and clean way to get around it, as votemanager depends on some further modification of the tables and unpacking them later to work properly. Turning the vote reason into an empty string "" sounds like a rather small sacrifice to ensure proper functionality.

* votemanager: add additional voteban settings for server owners

banPlayer provides a lot of banning options, none of which votemanager currently supports changing. This commit adds settings to the meta.xml for the following settings: voteban_banip true/false (default: true); voteban_banusername true/false (default: false); voteban_banserial true/false (default: false); voteban_duration number in seconds (default: 3600)

* votemanager: add additional voteban settings for server owners #2

addendum to previous commit

banPlayer provides a lot of banning options, none of which votemanager currently supports changing. This commit adds settings to the meta.xml for the following settings: voteban_banip true/false (default: true); voteban_banusername true/false (default: false); voteban_banserial true/false (default: false); voteban_duration number in seconds (default: 3600)

* votemanager: store empty string for vote reason instead of nil if none is provided addendum

Removed unnecessary reason == "" from kill votes as it is never used after the initial use.

* changed default voteban behavior from IP to serial
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants