Add a blacklist to block cheaters #135
Conversation
|
Can you disable auto-format in your visual studio and undo all the changes that are only format changes? That would make the PR a bit easier to read. It looks like this does not prevent DSCM from creating connections to blocked players or did I just miss that? |
|
Ah, my bad! Fixed up and squashed into a single commit. |
|
Thx for undoing the formatting
|
DaS-PC-MPChan/MainWindow.vb
Outdated
| If dsProcess IsNot Nothing Then | ||
| For Each entry As DataGridViewRow In dgvBlockedNodes.Rows | ||
| Dim steamid As String = entry.Cells(1).Value | ||
| dsProcess.DisconnectSteamId(steamid) |
There was a problem hiding this comment.
This line will start a thread in the Dark Souls Process and send a packet to the user with the given steamID. It does this independent of whether we are actually connected to that user. This seems bad.
I am also not sure whether there are race conditions with this – starting 5 of these threads at once might cause problems.
Disconnects should only be triggered for nodes that we are actually connected to and then only one at a time.
There was a problem hiding this comment.
How do you feel about
If dsProcess IsNot Nothing Then
For Each blockNode As DataGridViewRow In dgvBlockedNodes.Rows
For Each connectedNode As DataGridViewRow In dgvMPNodes.Rows
Dim blockSteamId As String = blockNode.Cells(1).Value
Dim connectedSteamID As String = connectedNode.Cells(1).Value
If (blockSteamId.Equals(connectedSteamID)) Then
dsProcess.DisconnectSteamId(blockSteamId)
End If
Next
Next
End If
This fixes the issue with disconnecting non connected users. Although i'm not sure what you mean by disconnects should "only one at a time".
|
This PR breaks the UI. The UI is broken both in it's default size (the buttons at the bottom don't fit) and when not expanded (click the "Expand DSCM") checkbox). |
|
Found a bug where blocks aren't removed from the registry when you press the remove block button |
Any suggestions as to fixing this? I can't make the buttons smaller without cutting off text, and i can't move the buttons/items to the left or right without overlapping other elements. |
|
Sry, I actually don't have a lot of time to look at this at the moment. I will try and have another look, when I'm less swamped for work. Or maybe wulf will find some free time first. |
Not a problem, no rush! |
|
Apparently this already exists: https://github.com/eur0pa/DSNA |
|
Yeah, just saw myself. I had no idea about that when I made this, whoops 😌! So, do we want to have this feature built into dscm, or keep it separate? I personally think the former is better, since it's one less program to run, plus it fits in with dscm's purpose of network management anyway. |
|
I agree that having this in DSCM would be nice, but I do prefer euro0pa's implementation for the blocking (=actually blocking connections to blacklisted nodes). Your implementation can actually be quite easily circumvented by a cheater. |
|
Easily circumvented? How so? The only way i can see is if they connect and invade in the 2 seconds between disconnections. But i'll take a look at euro0pa's implementation and try and port it over. |
|
Ah wait, I see what you mean. The Disconnect function only sends a request to the other player to disconnect, which they can ignore. |
|
Also, it appears the |
|
(still WIP, just pushing) |
|
The fact that |
|
Oh, sorry, my bad for assuming. I believed it's a bug because it breaks my current code and any other assembly with multiple returns. By appending the original instructions, if you have a second code section that jumps back to the original location, or just returns from the function (like mine), then those original instructions are not executed for that case. |
|
However, since this is desired behaviour, I'll change my fix to make prepending it require an optional argument (if that's ok) |
|
I don't quite understand why the original behavior is a problem for your case. If you |
|
Well, for my case i'd have to refactor my code to use esp instead of ebp, but the more important consideration is if the overwritten instructions write to memory or have some side effects. |
|
The idea is that you hook into the location of the instruction before which you want to run. If you want an instruction to run before your code, just hook later. Running the code you replaced before or after the injected code has exactly the same results – the only thing that changes is which hook-location you have to specify. The reason why it makes more sense to run the replaced code at the end is that this allows you to hook into the beginning of a function which would otherwise be impossible. Since placing code after a And isn't that actually exactly what you want to do here, but since you can't hook into the begging of the function with your method, you have to undo what the beginning of the function does with these lines which you could just remove if you used the original hook method. But it's also late here and I might just be completely missing what's going on :) But is there a reason why can't just move the hook location by 6 bytes, use the original hooking method and change nothing else? Btw, in case you care: you can simplify the comparison loop like this: |
|
Oh huh, yeah that makes a lot of sense. I've just always used Cheat Engine, which does it the prepend way, so i got used to that. Thank you for explaining this! I don't think i can use the append method without modifying my code to use esp instead of ebp, but i'd be happy to fix that. But i see now that both are valid approaches, i'm just used to doing it this way :) Also GAH, i knew i was overcomplicating that injection, thanks for the tip. I'll fix that up. |
* User can select up to 5 other players to block * Blocking done via injections inst SteamAPI ReadP2PPacker and SendP2PPacket
|
Right, there we go; all fixed up, improved to be non-bypassable, working, and squashed! |
|
Could you make sure DSCM never selects a blocked node as a good candidate, please? For this you need to add the blocked nodes to the |
|
Oh wait, the gui bug does appear when Expand DSCM isn't checked, i see. I can see about fixing that |
|
@Chronial Fixed the gui bug and DSProcess doesn't access MainWindow anymore. |
…Also add aob check at the injection site to detect this if it happens again.
…ant blocking a user to always work.
…e blocklist features before the game has finished loading the steamapi dll
-Add UI Features -Have an async function run in UpdateNodes whenever a new node connects -Add config setting -Add function to lookup date of user account creation -Add function to block user based on user account creation
…ion between DSCM runs
…re for the "testing" update branch
…t cut off so easily
|
since this version of DSCM isn't getting updated again according to the newest commit, i'll just close this PR. My fork with these changes will be available + kept updated. |
Following Hillomunkki's work, I've implemented a blocking system in DSCM. Users can block up to 5 other players, and blocks are stored in the registry.
Implementation wise, blocking is done via the same method as Europa's https://github.com/eur0pa/DSNA:
Injecting code into the SendP2PPacket and ReadP2PPacket steamAPI functions.
This resolves issue #122