Recent Exploit Fix

Naltalah · 01/11/2021, 11:54

Since there were some attacks going on last night on several servers abusing uninitialized CUser objects in CDPSrvr, I tried coming up with a fix for it. I just got it working when making my client spam the abused function every 100ms on login. Since it affects multiple servers, I want to share it. No guarantee this is a fix for it, but it worked for me. If you have suggestions about it or concerns, please post here or pm me.

IMPORTANT UPDATE:
This fix will NOT fully fix the attacks that were going on previously. This fix will only filter messages from invalid players sending packets.
The actual fix has been developed with a lot of outside help and in a group of 10 people.
The group in total has decided to not disclose any details as we invested a lot of time into it and simply don't want to share it with everyone out there.
Sharing a fix will also mean making servers that are not present on epvp vulnerable to the attack. We don't want that to happen.

The invalid user filter will stay on here, but nothing else will be disclosed.

Previous Updates:

Spoiler

User.h
below

Code:

map<DWORD, CUser*> m_users;

add

Code:

map<DWORD, CUser*> m_invalid_users;

below

Code:

    CUser* GetUser(DPID dpidCache, DPID dpidUser);

add

Code:

    CUser* GetInvalidUser(DPID dpidUser);

User.cpp
add

Code:

CUser* CUserMng::GetInvalidUser(DPID dpidUser)
{

    auto it = m_invalid_users.find(dpidUser);
    if (it != m_invalid_users.end())
        return it->second;
    else
        return nullptr;
}

in

Code:

CUserMng::AddPlayer

add at the top of the function

Code:

    m_users.insert({ pUser->m_dwSerial, pUser });
    m_invalid_users.erase(pUser->m_dwSerial);

In

Code:

CUserMng::AddUser

replace

Code:

         m_users.insert(std::make_pair(dpidUser, pUser));

with

Code:

        m_invalid_users.insert({dpidUser, pUser});

in

Code:

CUserMng::RemoveUser

at the top of the function, add

Code:

    auto findInvalid = m_invalid_users.find(dwSerial);
    if (findInvalid != m_invalid_users.end())
    {
        m_invalid_users.erase(findInvalid);
        return;
    }

DPDatabaseClient.cpp

In OnJoin replace

Code:

     CUser* pUser = g_UserMng.GetUser( dpidCache, dpidUser );

with

Code:

     CUser* pUser = g_UserMng.GetInvalidUser( dpidUser );

DPSrvr.cpp

In UserMessageHandler after GETTYPE( ar ) add

Code:

    CUser* pUser = g_UserMng.GetInvalidUser(*(UNALIGNED LPDPID)lpMsg);
    if (pUser)
    {
        Error("Received Packet from Invalid user");
        return;
    }

in OnAddUser
after

Code:

	if( nSlot >= 3 )
		return;

add

Code:

    CUser* pInvalidUser = g_UserMng.GetInvalidUser(dpidUser);
    if (pInvalidUser)
    {
        Error("Invalid Connection Attempt UserID [%u]", idPlayer);
        return;
    }

Consider removing the Errors since multiple logins may work server lag with too many calls. It's mostly for debugging purposes.

After adding that the error triggered twice when logging in local server. As said above, if you have concerns or suggestions on how to improve that code, post here or pm me.

Quick demonstration of the fix:

Contributions and suggestions made by:
Vladi#5038
Dylan#8888
kotori#0001

miniman06 · 01/14/2021, 20:12

Great work, minor notes

Code:

 m_users.insert({ pUser->m_dwSerial, std::move(pUser) });

move isn't strictly needed on raw pointers.

Code:

m_invalid_users.insert(std::make_pair(dpidUser, pUser));

make_pair can be shortened with {} initializer list, as in previous note.

xpeurbill · 01/14/2021, 21:41

In the function `CUserMng::GetInvalidUser` you never use the variable `DPID dpidCache`.

Replace :

Code:

CUser* GetInvalidUser(DPID dpidCache, DPID dpidUser);

by:

Code:

CUser* GetInvalidUser(DPID dpidUser);

Code:

CUser* CUserMng::GetInvalidUser(DPID dpidCache, DPID dpidUser)
{
    UNUSED_ALWAYS(dpidCache);

    std::map<DWORD, CUser*>::iterator it = m_invalid_users.find(dpidUser);
    if (it != m_invalid_users.end())
        return it->second;
    else
        return NULL;
}

by:

Code:

CUser* CUserMng::GetInvalidUser(DPID dpidUser)
{

    std::map<DWORD, CUser*>::iterator it = m_invalid_users.find(dpidUser);
    if (it != m_invalid_users.end())
        return it->second;
    return nullptr;
}

Naltalah · 01/15/2021, 05:03

Updated original post with the feedback, most of the code-style was just copy-paste, but I agree std::move isnt needed. In theory if we were to adhere to good practice, I would use auto for iterator types and use nullptr literal to return a nullptr, there are still some people using vs2003 tho (for some ***-forsaken reason) so that's why I usually stick to old styles when releasing something. Shouldn't probably do that.

TheCrawler · 03/01/2021, 06:45

Sadly this is not enough for attacks sent directly to cacheserver

Vltimus · 09/19/2024, 20:46

Is there any new fix in 2024 that works properly in blocking this?