Pointer problem!

Hi guys, how are you? Im new here so if i have to introduce myself somewhere in the forum just tell me!

I'm having a little problem with my code.

The situation:
I have an online RPG game source which send a lot of data between server files and client.

This game has a security breach which allow malicious people with a packet editor like "wpe" to edit the packets sent to server, resend the packet continuously, etc.

To fix this I want to put a packet id before the data, but i'm doing something wrong and i don't know what is going wrong.

This the original code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
// cData is the msg that will be sent to the other server files or to client.
// I want to insert the packet ID before cData
int XSocket::iSendMsg(char * cData, DWORD dwSize, char cKey)
{
 WORD * wp;
 int    i, iRet;

	cKey = 0;

	if (dwSize > m_dwBufferSize) return DEF_XSOCKEVENT_MSGSIZETOOLARGE;

	if (m_cType != DEF_XSOCK_NORMALSOCK) return DEF_XSOCKEVENT_SOCKETMISMATCH;
	if (m_cType == NULL) return DEF_XSOCKEVENT_NOTINITIALIZED;

        // Key input
	m_pSndBuffer[0] = cKey;

        // Packet size
	wp  = (WORD *)(m_pSndBuffer + 1);
	*wp = dwSize + 3;

	memcpy((char *)(m_pSndBuffer + 3), cData, dwSize);

        // Encrypt
	if (cKey != NULL) {
		for (i = 0; i < dwSize; i++) {
			m_pSndBuffer[3+i] += (i ^ cKey);
			m_pSndBuffer[3+i]  = m_pSndBuffer[3+i] ^ (cKey ^ (dwSize - i));
		}
	}
	iRet = _iSend(m_pSndBuffer, dwSize + 3, TRUE);

	if (iRet < 0) return iRet;
	else return (iRet - 3);
}


This is the new method:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53

// cData is the msg that will be sent to the other server files or to client.
// I want to insert the packet ID before cData
int XSocket::iSendMsg(char * cData, DWORD dwSize, char cKey)
{
 WORD * wp;
 int    i, iRet;

	cKey = 0;

	if (dwSize > m_dwBufferSize) return DEF_XSOCKEVENT_MSGSIZETOOLARGE;

	if (m_cType != DEF_XSOCK_NORMALSOCK) return DEF_XSOCKEVENT_SOCKETMISMATCH;
	if (m_cType == NULL) return DEF_XSOCKEVENT_NOTINITIALIZED;

        // Key input
	m_pSndBuffer[0] = cKey;

        // Packet size
	wp  = (WORD *)(m_pSndBuffer + 1);
        // Should I increase dwSize to + 7?
	*wp = dwSize + 3;

        // I've tried to insert the packet data here doing this:

        int *ip; // int pointer.
        // Everytime the function is called increase m_iPacketID
        // m_iPacketID is a XSocket class member
        m_iPacketID++;
        // Restart the packetID
        if (m_iPacketID > 20000) m_iPacketID = 0;
        // Should ip point to m_pSndBuffer + 3?
        ip  = (int *)(m_pSndBuffer + 3);
        // Insert m_iPacketID into m_pSndBuffer +3 address
        *ip = m_iPacketID;

        // Should I copy the data to m_pSndBuffer + 7 address?
        // Copy the data to m_pSndBuffer +7 instead of +3
	memcpy((char *)(m_pSndBuffer + 7), cData, dwSize);

        // Encrypt
	if (cKey != NULL) {
		for (i = 0; i < dwSize; i++) {
			m_pSndBuffer[3+i] += (i ^ cKey);
			m_pSndBuffer[3+i]  = m_pSndBuffer[3+i] ^ (cKey ^ (dwSize - i));
		}
	}
        // dwSize + 3 -> dwSize + 7
	iRet = _iSend(m_pSndBuffer, dwSize + 7, TRUE);

	if (iRet < 0) return iRet;
	else return (iRet - 3);
}


With the new one i'm loosing packets, i dont know why. Sometimes server and client connects and most times it doesn't.

I'm sure the problem is how i'm using the pointers.

What i'm doing wrong?

Thanks in advance!
1
2
3

        // Restart the packetID
        if (m_iPacketID > 20000) m_iPacketID = 0;


How is the packet id handled on the receiving side? If the receiver gets a packet id of 2 and then a 19998 comes in, what is done?

There is going to be two side to this, so you also need to look at the client code for connections and packet handling.
Last edited on
The game itself in the server side is a bounch of .exe that connects between themselves & client side is another exe that connect to those files on the server side.

Client and server side uses the same connection lib that handles how the data is received and sent.

This first method receive the data. It doesn't has packet ID handling here.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
int XSocket::_iOnRead()
{
 int iRet, WSAErr;
 WORD  * wp;	

	if (m_cStatus == DEF_XSOCKSTATUS_READINGHEADER) {
		
		iRet = recv(m_Sock, (char *)(m_pRcvBuffer + m_dwTotalReadSize), m_dwReadSize, 0);	
		
		if (iRet == SOCKET_ERROR) {
			WSAErr = WSAGetLastError();
			if (WSAErr != WSAEWOULDBLOCK) {
				m_WSAErr = WSAErr;
				return DEF_XSOCKEVENT_SOCKETERROR;
			}
			else return DEF_XSOCKEVENT_BLOCK;
		}
		else 
		if (iRet == 0) {
			m_cType = DEF_XSOCK_SHUTDOWNEDSOCK;
			return DEF_XSOCKEVENT_SOCKETCLOSED;
		}
		
		m_dwReadSize      -= iRet;
		m_dwTotalReadSize += iRet;
		
		if (m_dwReadSize == 0) {
			m_cStatus = DEF_XSOCKSTATUS_READINGBODY;
			
			//wp = (WORD *)(m_pRcvBuffer + 1); // Original
                        // Now Packet size is at m_pRcvBuffer +5 address instead of +1
                        wp = (WORD *)(m_pRcvBuffer + 5);
			m_dwReadSize = (int)(*wp - 3);
			
			if (m_dwReadSize == 0) {
				m_cStatus        = DEF_XSOCKSTATUS_READINGHEADER;
				m_dwReadSize      = 3;
				m_dwTotalReadSize = 0;
				return DEF_XSOCKEVENT_READCOMPLETE;
			}
			else 
			if (m_dwReadSize > m_dwBufferSize) {
				m_cStatus        = DEF_XSOCKSTATUS_READINGHEADER;
				m_dwReadSize      = 3;
				m_dwTotalReadSize = 0;
				return DEF_XSOCKEVENT_MSGSIZETOOLARGE;
			}
		}
		return DEF_XSOCKEVENT_ONREAD;
	}
	else
	if (m_cStatus == DEF_XSOCKSTATUS_READINGBODY) {
		
		iRet = recv(m_Sock, (char *)(m_pRcvBuffer + m_dwTotalReadSize), m_dwReadSize, 0);
		
		if (iRet == SOCKET_ERROR) {
			WSAErr = WSAGetLastError();
			if (WSAErr != WSAEWOULDBLOCK) {
				m_WSAErr = WSAErr;
				return DEF_XSOCKEVENT_SOCKETERROR;
			}
			else return DEF_XSOCKEVENT_BLOCK;
		}
		else 
		if (iRet == 0) {
			m_cType = DEF_XSOCK_SHUTDOWNEDSOCK;
			return DEF_XSOCKEVENT_SOCKETCLOSED;
		}

		m_dwReadSize      -= iRet;
		m_dwTotalReadSize += iRet;
		
		if (m_dwReadSize == 0) {
			m_cStatus        = DEF_XSOCKSTATUS_READINGHEADER;
			m_dwReadSize      = 3;
			m_dwTotalReadSize = 0;
		}
		else return DEF_XSOCKEVENT_ONREAD;
	}

	return DEF_XSOCKEVENT_READCOMPLETE;
}


This method is called by the core source to get the data that was sent by the client (that could be the gameclient.exe by himself or each of the server .exe)

Original:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
char * XSocket::pGetRcvDataPointer(DWORD * pMsgSize, char * pKey)
{
 WORD * wp;
 DWORD  dwSize;
 register int i;
 char cKey;
	
	cKey = m_pRcvBuffer[0];
	if (pKey != NULL) *pKey = cKey;

	wp = (WORD *)(m_pRcvBuffer + 1);
	*pMsgSize = (*wp) - 3;
	dwSize    = (*wp) - 3;

	if (cKey != NULL) {
		for (i = 0; i < dwSize; i++) {
			m_pRcvBuffer[3+i]  = m_pRcvBuffer[3+i] ^ (cKey ^ (dwSize - i));
			m_pRcvBuffer[3+i] -= (i ^ cKey);
		}
	}
	return (m_pRcvBuffer + 3);
}


New method
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
char * XSocket::pGetRcvDataPointer(DWORD * pMsgSize, char * pKey)
{
 WORD * wp;
 DWORD  dwSize;
 register int i;
 char cKey;
	
	// Get the encryption
	cKey = m_pRcvBuffer[0];
	if (pKey != NULL) *pKey = cKey;

	// Get the packet size
	wp = (WORD *)(m_pRcvBuffer + 1);
	// Should I change -3 to -7?
	*pMsgSize = (*wp) - 3;
	dwSize    = (*wp) - 3;
	
	// Packet id handling
	int *ip;
	
	// Pointing to m_pRcvBuffer +3 address
	ip = (int *)(m_pRcvBuffer + 3);
	// Getting the data
	*ip = m_iPacketID;
	
	// TODO: add block packet handling here
	// m_iTempPacketID is previously initialized to 0
	if (m_iPacketID == m_iTempPacketID) {
		PutLogList("Packet blocked");
		// Do something here
	}
	else m_iTempPacketID = m_iPacketID;
		

	// What i should do here with the encryption?
	if (cKey != NULL) {
		for (i = 0; i < dwSize; i++) {
			m_pRcvBuffer[3+i]  = m_pRcvBuffer[3+i] ^ (cKey ^ (dwSize - i));
			m_pRcvBuffer[3+i] -= (i ^ cKey);
		}
	}
	
	// returns the data called from the source core.
	return (m_pRcvBuffer + 3);
}


This is the call to pGetRcvDataPointer from core source
1
2
3
4
5
6
7
	case DEF_XSOCKEVENT_READCOMPLETE:
		pData = m_pClientList[iWorldH]->m_pXSock->pGetRcvDataPointer(&dwMsgSize, &cKey);
				
		if (bPutMsgQuene(DEF_MSGFROM_LOGSERVER, pData, dwMsgSize, iWorldH, cKey) == FALSE) {
			PutLogList("@@@@@@ CRITICAL ERROR in LOGSERVER MsgQuene!!! @@@@@@");
		}
		break;


I need insert the packetID before the data sent by the server / client and move the data 4 positions in memory. I've tried everything but it still loosing packets so i'm sure that i'm doing it wrong.

thanks!
OK, I don't see anything up front that is wrong. I would need to run it through a debugger and watch the values change.

I'm not sure about this part:

1
2
3
4
5
	// Pointing to m_pRcvBuffer +3 address
	ip = (int *)(m_pRcvBuffer + 3);
	// Getting the data
	*ip = m_iPacketID;
	


Don't you end up over writing the message ID that is sent to you with the one that is local to the machine? This is a receiving function and other than the decryption loop, you should be reading from it, not writing to it.
Last edited on
Hi roberts! thanks for your reply and your time! When the socket class receive the data it insert the packetID into another TempPacketID var.

I was checking a little more and I've found that one of those executables had a socketclass.lib linked into the project, that's why the code didn't work! I made a few changes in the socket class.cpp that i had and now is working great, also block the packets edited by this packet editor.

Thank you so much for your help!
Topic archived. No new replies allowed.