kakysha / HonorSpy

World of Warcraft: Classic HonorSpy addon
57 stars 50 forks source link

Users Spoofing data #58

Closed DonTheCrown closed 1 year ago

DonTheCrown commented 4 years ago

Currently users are somehow able to spoof data into the ranking with no ability to remove/fix it. For example currently the rank 1 of Grobbulus-NA Alliance is listed as "All of " with 420k honor and 133337 RP,

honorSpyIssue

jesse-greathouse commented 4 years ago

They're hacking the HonorSpy code to broadcast false information on the sync channel. They're not doing anything wrong, per se, it's just not nice. It may be possible to build a blacklist feature into HonorSpy, so if someone know's exactly who is being malicious, they can add them to the blacklist, and their client would ignore broadcasts from that character. However, the way the HonorSpy code works, it seems that other people, who do not have them blacklisted, could still propagate the false information.

You could potentially create a feature that would remove them from the standings list, and remember who was being removed, but in this case they are not even using the name of a real player and could potentially just keep changing the name of who's on the list.

It may be possible to verify that the names on the standing list are real characters, but that would potentially involve querying each name that gets submitted which would cause mass query spamming, and opens the door for real exploitation.

I think the best thing to do, in this case, is just ignore the phony data.

sasha-koss commented 4 years ago

An extra Tab with ignored Values and Broadcasters would be nice. The "Next Week Rank" feature is pretty nice to see if you make the cutoff but the spoofing destroys it.

kakysha commented 4 years ago

@jesse-greathouse thanx for your commentary, thats exactly what I was thinking. Sadly, there is no way to check for specific playername to exists on the server. Thats why the best what we all can do, is just to ignore it.

Probably I can add "remove" feature locally, but not sure how many ppl will use that.

DonTheCrown commented 4 years ago

my concern is that next week there is going to be 100 entries like this on my realm and that's going to make honorspy worthless all it takes is for 1 more person to figure it out and the troll war begins

kakysha commented 4 years ago

I did implemented a little bit of filtering for special symbols in player names. but that's nothing. Lets see where it will come first. If its going to be more than couple of kiddies trolling, then I probably implement local row remove feature.

cannonpalms commented 4 years ago

You can require two votes for players above a certain standing. It stands to reason that in order for a player to be above a certain point, a player has to be playing quite a lot. If someone is playing quite a lot, you should certainly see him appear in at least two player's shared data. Additionally, this could be bypassed if said player has the addon installed. Until a player above this arbitrary standing is seen twice, he is tracked in a separate table that is included in the payload when the database is shared. Include collection of the players GUID to add an extra layer of required coordination between two bad actors.

You may also consider adding constraints to RP, rank, and standing.

kakysha commented 4 years ago

Why you want to guard top players and allow any lower ranker? Both of this table parts are equally meaningful, so any type of consensus should apply to the whole table. But consensus is not an option here, since we have many players with enough HKs that was only spotted by one addon user.

Checking unrealistic values is possible, if we can find universal algorithm to estimate lower and upper bounds for all columns.

jesse-greathouse commented 4 years ago

Checking unrealistic values is possible, if we can find universal algorithm to estimate lower and upper bounds for all columns.

I don't think this is a bad idea, in theory, but I think any such algorithm will be exploitable. The thing I'm worried about is not the obvious person hacking the standings, but a more insidious falsification of the system where it's not transparent what is happening.

For instance, if any algorithm were to dynamically determine what the upper end and the lower end of the range should be, a clever hacker would pad the middle of the range with a dynamic number of fake entries, to expand the range on the upper and lower. I think our Clover Club hackers may not be sophisticated enough to figure this out, however someone who is sophisticated, could be hacking the system and no one would realize why their numbers are skewed.

At the end of the day, this makes very little difference so I don't have any desire to change or not change either way. But I agree with taking time to see if this problem even becomes a big issue, because right now it's not really effecting the community all that much. Adding smartness of this nature is going to be a big burden for any one person to take on. Ideally it would require a lot of testing to get right, and such testing is extremely difficult to do. I think your addon is brilliant @kakysha, but I don't envy the idea of adding this much complication to it. It sounds really hard to get right.

kakysha commented 4 years ago

Yes, the simplicity is what I aim for. Starting this endless war with script-kiddies can just do more harm than profit to all of us. Note for future readers: I considered many approaches how can we eliminate this "bad" data, and all of them brought me to dead ends or super complex solutions. There are no feasible ways of checking for the existence of specific player names, neither we can reach any consensus on "correct" distributed database state.

cannonpalms commented 4 years ago

I suggested a solution that places different safeguards on high and low ladder positions because the people spoofing are doing it for attention, thus want to be on top, and consensus is possible at such a high ladder position. It is impossible for someone legitimately # 1 on the ladder to only be seen by one addon.

The only malicious activity that my strategy does not prevent is spoofing hundreds of entries low in the table to affect the size of the ranked pool, and thus brackets. However, the current solution (none), also has this issue. Consensus can help keep the ladder accurate near the top where the heaviest power users are.

kakysha commented 4 years ago

How will you count "seen" times in one's addon? What can prevent a malicious person to create 10 characters and resend same bad data from all of them, so other players would think it was seen 10 times?

cannonpalms commented 4 years ago

You raise valid points. Short of a stand-alone client, (which btw—I’m emailing you about separately), this is all an exercise in security theater. I’m merely suggesting additional hoops.

Sent from my iPhone

On Dec 1, 2019, at 11:01 AM, kakysha notifications@github.com wrote:

How will you count "seen" times in one's addon? What can prevent a malicious person to create 10 characters and resend same bad data from all of them, so other players would think it was seen 10 times?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

Dieselwarden commented 4 years ago

As mentioned in my twitch message @kakysha I have a few suggestions that may hopefully help mitigate some of these issues:

Create a cap to the Ranking points at 100,000. Create a character limit of the max characters that can be used in a name. I believe it is 14 or 16. Create a function to remove entry ranges. (For example: on my server ranges 1- 1986 are filled with fake characters and data) -OR- If the player's name was logged to remove and ignore all further entries from

obviously this will not 100% fix the issue, however it will make it way more restricting. Hopefully some of these help, and if I can think of any other ideas I will share here or on twitch.

Dieselwarden commented 4 years ago

["RP"] = 1234567890, <--- not sure if there is an actual cap, but this number is not obtainable now. ["class"] = "DRUID", ["last_checked"] = 1575141695, ["lastWeekHonor"] = 123456789, <--- would set hard cap of 500k-1m ["standing"] = 1, <--- only 1 player can be in the identical standing i believe, but maybe no > 2? ["unitID"] = "player", ["rankProgress"] = 0, ["thisWeekHonor"] = 1.0e+34, <----- Force whole numbers & hard cap of 500k-1m ["rank"] = 16, <---- As far as I know there can only be 1 GM/HWL, would set a limit on that.

Just some more thoughts.

kakysha commented 4 years ago

Setting limits won't prevent hackers of creating 1000 "valid numbers". They won't be at the top, but still be in the table. No difference. "Remove entries" button won't fix your table either, as malicious person can create 500 entries that look valid to you and you won't even know that such players do not exist. As I said before, its a cats and mice game, in which there is no guaranteed way of defending against it. I don't want to overcimplicate addon, just to make "spoofing" a bit harder.

Dieselwarden commented 4 years ago

encrypt data tables so that they cannot easily be messed with? I mean someone can reverse engineer that too, but not just your average player.

kakysha commented 4 years ago

Thats why I posted a note for future readers before closing this: Note for future readers: I considered many approaches how can we eliminate this "bad" data, and all of them brought me to dead ends or super complex solutions.

Dieselwarden commented 4 years ago

Possible to vaidate player true/false via /who function?

Dieselwarden commented 4 years ago

could scan /who for all the names that will be updated into everyone's list on dead. if those names are not online, it doesnt push the data. sure some of the data will be lost, but it wont be able to push fake data if it cant get past an "online" check.

jesse-greathouse commented 4 years ago

could scan /who for all the names that will be updated into everyone's list on dead.

Querying people's names via /who is rate-limited to 1 name per 5 seconds. You can test this by creating a macro with /who [name]. You can spam it as much as you want but it will only return a result every 5 seconds. If you do it with multiple names it will only return the first name.

The idea of doing this was previously discussed, in the beginning of this thread, last week. If you can figure a way around the technical limitations of doing this, and submitting a pull request for it, I can't speak for @kakysha but personally I would be interested in seeing it.

Dieselwarden commented 4 years ago

Anything is better than what i just did going line-for-line scrubbing the whole list of fake and/or suspicious entries after setting it to sync over guild and logging out. I'll keep brainstorming, A setting that can choose to only sync data for names that you have already seen and specifically update the data for those names, without adding new name entries would prevent spoof names, but it would not prevent people from editing data for names that are already known unless you could opt out of syncing altogether.

Dieselwarden commented 4 years ago

@kakysha Another thought came to me. And I think this may be your fix. If you can log the character name of the player who sent info updates through chat, you could add a feature to ignore all info from "player name" so it would delete any info they sent and prevent future info from that same player. logic being, you could see who sent the erronius info and block any info from that player.

Example, if xjkeldfgoxh sent data to everyone on the server, their info would be tagged with the name, regardless of how the data was aquired. So if Monster sent data to everyone on the server because they are using the HonorSpy without malicious intent, but they have the data from xjkeldfgoxh who is a malicious dbag, xjkeldfgoxh would have the data tag, so when you ignore them, any info that Monster sent would still be sent, but xjkeldfgoxh's data would be ignored.

kakysha commented 4 years ago

And then Monster decides to become bad guy also, changes the "data tag" of bad rows from "xjkeldfgoxh" to "toatllyagoodplayer" and voila - everyone accepts this bad data thinking it came from good guy.

Dieselwarden commented 4 years ago

Right, but then you just add toatllyyagoodplayer to ignore and all the data tagged from that guy is excluded from the data set as well.

kakysha commented 4 years ago

And them Monster decides to replace toatllyyagoodplayer with every player name from his server, and eventually the whole server get banned.

Dieselwarden commented 4 years ago

its a possibility, but its better than having an addon that no one wants to use because its filled with thousands of lines of fake data. or have the ignore list reset every tuesday with the honor reset.

Dieselwarden commented 4 years ago

i see the fix i suggest as being a massive deterrant because it would be so time consuming for people to fake data. They are looking for low effort means to fake the data. going in and indivually adding actual player names would take them days to months worth of effort, and by that time the honor would reset. You could also add a feature that removes all data that is older than 3 days old, chances are after 3 days of not logging in either the data is fake or that person will not be pushing honor, and if they log in on the 4th day, they will be re-added.

kakysha commented 4 years ago

implementing an "ignore" feature means that there is one more dropdown menu, a separate GUI to add / delete / modify this "ignore" list, and this also means that if malicious person generates 1000 bad rows with 1000 random authors for each row, every player then should click each of this 1000 rows and click "ignore", as it's authors differerent for every row.

And no, it won't take days or months to ban the whole server. Ppl already generate fake data using scripts. It takes 5 minutes to get real player names from addon database and paste them into fake data's author column

Dieselwarden commented 4 years ago

well, I thought it would be helpful to share my ideas on how to fix or at least partially fix the problem. I havent dabbled with any scripting in many years, as i used to mess around trying to make things better, but have lost most of that knowledge. I know that there is 2 courses of action that you as the dev can make. either adapt and overcome, or just let it happen and see your creation lose relevance.

DonTheCrown commented 4 years ago

wouldnt that just make it extremely easy for a troll level 1 character to set their honor to whatever they want? Basically handing the tools to trolls to kill this addon...

DanHazard commented 4 years ago

Love the addon but unless something is done to prevent this it's basically useless. Soon as people noticed how easy it was to manipulate the data on my server suddenly there's nearly 100 and counting bunk entries. I understand that the creator probably doesn't have a ton of time, but maybe just delist the addon if nothing is going to be done about it. It's mostly worthless atm.

jacobneill1994 commented 4 years ago

theres a dude trolling me on my server thats altering the tables, he suggested using GUID?

jacobneill1994 commented 4 years ago

also a quickfix to receive comms from other players (comes from Mired Hunter on Arugal PvP) open honorspy.lua in the addon folder, find the line "function HonorSpy:OnCommReceive(prefix, message, distribution, sender)" delete everything thereafter up until the functions end (include end). restart your game and clear your tables, now all the data will only be received from mouseover? and not from player broadcasts (its a slower table acquisition but prevents spoofing)

Js-2nd commented 4 years ago

I've got some ideas. Label source of each record into 3 types.

  1. [self] data inspected by yourself (where INSPECT_HONOR_UPDATE does) always correct since they are received directly from server
  2. [guild] data received from guild channel almost trustable if you can trust your guild
  3. [public] data received from public channel (yell, raid, instance chat...) not that trustable but lets keep it anyway

Then OnCommReceive should behave like

  1. when receive messages from guild, records which labeled [self] will be labeled as [guild] instead, others keep the same
  2. when receive messages from public, all records received will be labeled as [public]

Finally, the honor table can display data from 3 different data sets which we can select now.

  1. [self]
  2. [self] + [guild]
  3. [self] + [guild] + [public]
leothlon commented 4 years ago

Hi, i have fixed this issue and sent you a message on curse on the ideas i had. How i fixed it was: every data you recieve is added but tagged as confirmed=false.

If you however see the player yourself (mouseover inspect) then they are considered confirmed and gets confirmed=true,

If 50 people share to you the same player then they are also considered confirmed and gets confirmed=true.

and only players what have confirmed=true are displayd and counted towards brackets.

by doing this someone who wants to mess with the data will be forced to make 50 alts and with each of them get guild invite or go to a BG or die close to other players. so yea it can still be faked but its ALOT harder, as the validation is done for each person locally. (you only broadcast data that you consider confirmed aswell)

It means the adding of new data will be slower but after 1 day it should be pretty fine anyway. I've also thought about "whitelisting" your own guildmates so that all data you get ABOUT them you consider confirmed aswell (as you know its a real player for sure). Do not make it so all data from them is confirmed however, as then it would be easy for 1 person in a big guild to spoof data, then have all guildies spread the spoofed data and we would be back to square 1.

here is the code with my changes: honorspy.lua : https://paste.ubuntu.com/p/gQfrK9ZSds/ GUI.lua : https://paste.ubuntu.com/p/h5MCtzpqnj/ (ps. this is my first time writing lua so i'm guessing you can do it alot cleaner and better than me, but it works atleast) if you decide to use this please just add a credit to Leomage-Earthshaker EU :)

Kristoferhh commented 4 years ago

If 50 people share to you the same player then they are also considered confirmed and gets confirmed=true.

I'm assuming this would only work if people (or honorspy itself) would delete all the data, otherwise everyone would share the fake entries which would confirm them(?)

PQlse commented 4 years ago

If 50 people share to you the same player then they are also considered confirmed and gets confirmed=true.

I'm assuming this would only work if people (or honorspy itself) would delete all the data, otherwise everyone would share the fake entries which would confirm them(?)

If I understand correctly all data is false by default and to verify it you actually have to see that person in game and then he has to be seen by whatever amount of people the threshhold is set to. Since those fake entries are not real characters they will not be shared no.

The issue I have found using this now for around 14h is that it syncs super slow even with the threshold set to 1 and having 4 people using it. It also doesnt seem to sync retroactively but only syncs data to people who are online at the same time but not whatever you have found while they were offline. Perhaps that's just from the low amount of people using it idk.

leothlon commented 4 years ago

well any time you die it should send your entire db (that is confirmed). and yea, seeing as it works by inspecting it makes the update work exponentionaly. and if we can set all "known names" like people from your own guild to be auto confirmed as we know those are real players this will help ALOT as they will help to confirm eachother faster.

another thing i saw i had missed was that it only updates if the new last seen is newer than the old one, so people need to see you in order, i have done some tweaks so that old data still counts towards the seen count, but not updating the actual data

PQlse commented 4 years ago

another thing i saw i had missed was that it only updates if the new last seen is newer than the old one, so people need to see you in order, i have done some tweaks so that old data still counts towards the seen count, but not updating the actual data

This would explain why its so slow! Please share the fix :)

leothlon commented 4 years ago

ok so here's the new honorspy.lua file https://paste.ubuntu.com/p/nnpWzmMhCJ/

if you check at the top on line 12 you see

local requiredForConfirm = 8;

This is the amount of people who needs to share the same name before its considered confirmed (ofc seeing them yourself instantly confirms them) When trying it out with only a few people then you might have it low, but with many people using the addon this should be high, this is basically what configures how hard it is to spoof the data... lower numer = people are added to table faster but easier to fake. high number = slow to add people to table but harder to fake.

I have also playd around with the idea of "trusted people", with this i am thinking to let you add players you trust to not fake data, then ALL their shared info will automatically be set as confirmed, because we trust them, this is kind of like what i said before about guild, but as its not automatic it would still make it harder to abuse as you would need people to add you on it if you wanted to fake.

kakysha commented 4 years ago

Pls, make a proper Pull Request, don't share code using pastebin. This solution, howerer, sounds doubtful to me, as the number of players that were "seen" by 50 players, or even 10 players, are actually very low (only the top brackets who actively play and always online). Instead of just having overpopulated pools with fake entries, you will have much smaller pools than it is in fact.

Kristoferhh commented 4 years ago

How about instead of having to hover over someone to confirm them, you'd do a /who request to see if they're online, confirming them. I haven't used /who requests in addons before so maybe someone else would know if this could work or not.

I'm still not sure though how you would confirm that the honor that is being shared is correct.

leothlon commented 4 years ago

nah, not sure how its on your servers, but on my server the is a TON of people, and prettymuch everyone uses honorspy. so it would take like 4 hours of you going in and out of bgs in a major city before 50 people have seen you. also this number can ofc be lowered.

You could set the limit to 10 people, but then check the level of the player sharing the data. require it to be lvl 15+ for example and the person who want to fake data needs to make 10 alts, level them above lvl 15 then go and die in close proximity to 10+ people ( same people for every alt). so it can still be faked, but would require ALOT more work.

besides in 1 MC raid you will have been seen by atleast 20 people (from your guild). you could also have so when someone gets confirmed they get added to a seperate whitelist that persist through resets, so consecutive weeks they dont need to get seen by 50 people again.

@Kristoferhh you cant for sure, BUT seeing as it has to be a real person, faked data will get overwritten with propper data next time someone sees the player, so faked data should only last for afew hours before getting fixed again automatically

kakysha commented 4 years ago

can you guys test latest commit https://github.com/kakysha/HonorSpy/archive/classic.zip It should remove fake entries now. I check for players existence using friends system. Need more testing, as the whole idea is a bit-hacky, again.

Kristoferhh commented 4 years ago

Laggy as fk while it's deleting :p but it seems to be working, so good job 👍

kakysha commented 4 years ago

@Kristoferhh What do you mean by laggy?

Kristoferhh commented 4 years ago

I realize now that it's most likely lagging because the fake players' names are all random ascii symbols (at least on my server), so printing the name in chat makes the game freeze for half a second.

ramboozer commented 4 years ago

Using the beta you have. I get a bit laggy to...game stutters, I doubt it has anything todo with ASCII symbols it appears to me its because its adding 100000 people and then removing them??? https://puu.sh/ESzYL/36baa294d6.png Goes on for about 5 or more minutes

kakysha commented 4 years ago

@ramboozer you should not see any ".. added to friends" messages. Thanks for reporting, I'll try to hunt this down.

ramboozer commented 4 years ago

Awesome, as you can see I showed you what I saw in the screenshot from Puu.sh. However it does appear to also be deleting all the false data