Enhance password security

[bobsobol]

Hey Ron, LTNS. :

As long as you enforce strict password rules (ex; requiring MixEDcaSe and $yMb0Ls) then the chances your hashed passwords will be guessed are very slim, even if a hacker obtains your password salt.

That was my point. The PT client and protocol already enforces that users cannot type any character which isn't a-z, A-Z, 0-9. (unless it's a Korean character, but most of our keyboards can't produce those in game) and that the password not be more than 7 characters.

So the number of possible passwords is relatively small.

This is a major downfall in any password encryption in todays' fast CPU environment.

As for Client side or Server Side... I say Server side for the same reason as Ron. The server may be leaked with the database, but the client is certainly already leaked.

One solution to the problem of giving away the encryption may be a Private / Public key system. ie, the client combines a known server public key with it's private key to send. The server needs to use it's private key with the clients public key. So neither end has the ability to decrypt what it encrypted, only what was encrypted by the other. (see PGP, or even RSA)

 

[Vormav]

The PT client and protocol already enforces that users cannot type any character which isn't a-z, 0-9.

Fixed
PT also don't use upper case in ID and PASSWORD.

Still... any security is better than non. No security only attract unnecessary attention.

 

[jvfl]

Fixed
PT also don't use upper case in ID and PASSWORD.

Still... any security is better than non. No security only attract unnecessary attention.

then I suggest encrypting when sending to the server
and the server, decrypt and encrypt to another type,
insurance, is not it?

 

[SheenBR]

I see. Then maybe:

md5( md5(password) + salt )

should be more "secure". I have done the routines in MASM and C++. Using a separated DLL file responsible for hasing and patching the packet. What should I use, MASM or C++? Should I use a DLL or try to patch the code inside the game like vormav did with the external item table?

 

[jvfl]

I see. Then maybe:

md5( md5(password) + salt )

should be more "secure". I have done the routines in MASM and C++. Using a separated DLL file responsible for hasing and patching the packet. What should I use, MASM or C++? Should I use a DLL or try to patch the code inside the game like vormav did with the external item table?

in this case, i prefer inside game
though the dll is only encrypt,
but, inside game, think more secure

 

[bobsobol]

DLL, and C++ (if it makes no difference) for "my two cents".

Some things can be done in Asm which are convoluted in C++, like altering the return address when they are CALLed, as an example. At the point the C++ becomes mostly inline assembly, it's not worth using C++.

Otherwise, clear C++ which relies on no non-standard framework (like MFC or ATL what-ever) is preferable, because it will work with any C++ compiler, where most Assemblers require modification, at least in meta-statements. (stuff like .model and CODE SEGMENT which don't relate to instructions but direct the compiler, or linker)

 

[SheenBR]

The functions are ready. I choose to do in C++. The only problem Im facing now is to replace some game functions to my DLL... I think it'll be ready soon.

You can find the encryption routine here:

void EXPORT GenerateMD5(char* passwd)    
{ 
char* salt = "magic";
std::string result = md5(passwd);      
char hash[32];   
ZeroMemory(&hash,33);    
ZeroMemory(&finalHash,33);
for(int i=0;i<result.length();i++)   
    hash[i] = result[i];        
// append hash with salt using wsprintf      
char* buffer = (char*)malloc( lstrlenA(salt) + lstrlenA(hash) );   
ZeroMemory(buffer,lstrlenA(salt) + lstrlenA(hash));
wsprintfA(buffer,"%s%s",hash,salt);       
// hasing again  
std::string finalResult = md5(buffer);    
for(int i=0;i<finalResult.length(); i++)         
   finalHash[i] = finalResult[i];    
}

 

[bobsobol]

1) If hash is 32 char long, why ZeroMemory(&hash,33), does ZeroMemory() take length + 1? If so, the malloc() followed by ZeroMemory() should have a +1 in the algorithm. strlen() doesn't include '\0' char.
2) Consider

You must be registered to see links

instead of malloc() followed by ZeroMemory()?

For (1), IDK how ZeroMemory() is written, and I'm quibbling over 1 byte, but some OS, or OS changes can throw an exception if 1 byte is wrong.

For (2), again, I don't know how ZeroMemory() is written, and certainly the stdlib.lib includes are very poorly optimised in many MS C++ compilers. (I often replace them with an alternative)

 

[SheenBR]

ZeroMemory is a #define for this instruction:

#define RtlZeroMemory(Destination,Length) memset((Destination),0,(Length))

I declared hash with 32 length and used ZeroMemory with 33 to have one 0x00 in the end, to make the string null-terminated. But as you told to use calloc that initialize a block of memory filled by zeros, I dont think I will have to use ZeroMemory again in the buffer variable...

 

[Gregoo]

Instead of hashing twice, why don't you use the Keyed Hash algorithms ?
Like HMAC-SHA1 or HMAC-MD5 ?

It's basically what you're trying to achieve, just better ;]

You must be registered to see links

 

[SheenBR]

Because it needs .net framework?

 

[bobsobol]

The rest sounds good, but...

I declared hash with 32 length and used ZeroMemory with 33 to have one 0x00 in the end, to make the string null-terminated.

But you haven't (that I can see) declared that last byte. (you don't own it)

In C++ there is nothing to stop you overwriting memory which doesn't belong to you. Sooner or later, as a program grows, or runs for a long time, the program with crash, or the OS will take offence to it.

You are using hash as though it's

struct hash{
char letters[32];
char null;}

Because you didn't declare 33 elements (and I now see that ZeroMemory() works on bytes, which strictly speaking a char may not be... but that's another story) there is no telling what the CRT may decide to allocate to the 33rd byte you zero. If you declare a UBYTE somewhere, that my well be zeroed alongside. Especially as 32 bytes aligns perfectly to a 4 byte boundary. (all depends on the memory management in the CRT, changes with each compiler version)

Just a friendly warning.

@HMAC: I see it can be used from unmanaged code, and I think the Clan and SQL DLLs include bits of .Net framework 2. (which does support it) It would be nice to avoid greater dependency though, and apparently Windows 2000 doesn't support it. :S (either that, or they just don't list support on older OS)

 

[SheenBR]

@HMAC
you mean this?

You must be registered to see links

@bob
Maybe then thats the reason why I am getting some errors while trying to patch some game functions to include the hashed password in the packet... I'll try again later.

 

[Gregoo]

@HMAC: I see it can be used from unmanaged code, and I think the Clan and SQL DLLs include bits of .Net framework 2. (which does support it) It would be nice to avoid greater dependency though, and apparently Windows 2000 doesn't support it. :S (either that, or they just don't list support on older OS)

You must be registered to see links

You must be registered to see links

Minimum supported client: Windows 2000 Professional
Minimum supported server: Windows 2000 Server
Header: Wincrypt.h
Library: Advapi32.lib
DLL: Advapi32.dll

 

[SheenBR]

HA! I just found out why the code isnt working. The thing is: I am adding my codes, messing up with ESP register. The function I am patching uses the ESP register to get the packet location. I only need to add a few more bytes to the ESP register and the problem will be solved, working on that now.

@edit

Everything is working fine now, working on the encryption algo now. It will be ready for release in a few hours.

Somehow I cant add a full 32 length MD5 hash without causing the client to crash, I can only add a 23 length MD5 hash... IDK why. I was testing this in my C++ emu project. I had to deactivate the password validation, otherwise the client will oddly keep connecting and sending the login packet infinitely, emu error? IDK, thats why I ask for someone who can set up a local server (I had one, but somehow that P*S stopped working without notice lol) and try it.

Steps to install it in your client:

Add the dll and the two methods to the game.exe using the Import Adder tool of CFF Explorer.
Now we need to get the pointer to our two functions that we've added. Press the M button and search for security.dll inside your client modules.

044D8140  73 65 63 75 72 69 74 79 2E 64 6C 6C 00 00 00 47  security.dll...G
044D8150  65 6E 65 72 61 74 65 4D 44 35 00 00 00 50 61 74  enerateMD5...Pat
044D8160  63 68 4C 6F 67 69 6E 50 61 63 6B 65 74 00 8F 12  chLoginPacket.
044D8170  03 00 FE 11 03 00 00 00 00 00 4D 81 0D 04 5B 81  .þ.....M.[
044D8180  0D 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ...............

0x44D816E is the offset that points to our GenerateMD5 function.
0x44D816E + 4 is the offset that points to our PatchLoginPacket function.

*Notice that these values changes according to your client.*

Open your game.exe in olly, CTRL+S and search for these instructions:

XOR EAX,EAX
TEST ECX,ECX
SETE AL
RETN

Press CTRL+L until you find no more results, thats where you'll patch the codes.
You will reach here:

00558570  /$ 83EC 54        SUB ESP,54
00558573  |. 8B4424 58      MOV EAX,DWORD PTR SS:[ESP+58]
00558577  |. 3D 85004748    CMP EAX,48470085
0055857C  |. 56             PUSH ESI
0055857D  |. 8B7424 64      MOV ESI,DWORD PTR SS:[ESP+64]

Starting in the SUB ESP,54 patch these codes:

PUSH ESI
MOV ESI,3351F88 -> this offset holds the password
PUSH ESI
CALL DWORD PTR DS:[ptr to GenMD5]
ADD ESP,4
CALL DWORD PTR DS:[ptr to PatchLoginPacket]
POP ESI
RET

Finding the password offset:
Search for this command in olly:

PUSH 48470085

The first result will be something like:

00526403   > 68 881F3503    PUSH Retrieva.03351F88 -> HERE IS THE PWD OFFSET
00526408   . 68 88203503    PUSH Retrieva.03352088
0052640D   . 68 85004748    PUSH 48470085

Changing the source:

You need aswell change a few offsets in the source. Go back in that routine I told you to patch the new codes. Open the PatchLoginFunction in the dll source, you'll see it is pure ASM, copied from the routine we just patched. All you need to do is compare the routine in olly and in the source. The instructions you need to fix in the source:

fBla Unknow = (fBla)0x50DDB0;
fSend SendPacket = (fSend)0x44D160;

MOV ECX,DWORD PTR DS:[0x345DEBC] -> line 73
PUSH 0x034309F0 -> line 84

MOV DWORD PTR DS:[0x34309F4],0x48470023 -> line 86
MOV DWORD PTR DS:[0x34309F0],0x18 -> 87
MOV DWORD PTR DS:[0x34309FC],-19 -> 88
MOV DWORD PTR DS:[0x34309F8],0 -> 89
MOV DWORD PTR DS:[0x3430A04],2 -> 90

The SendPacket offset is this call: (below the codes we just patched)

005585E0   . E8 7B4BEFFF    CALL back.0044D160
005585E5   . 5F             POP EDI
005585E6   . 5E             POP ESI
005585E7   . 83C4 54        ADD ESP,54

The Unknow offset is this call: (below the codes we just patched)

00558609   . C705 FC094303 >MOV DWORD PTR DS:[34309FC],-13
00558613   . C705 F8094303 >MOV DWORD PTR DS:[34309F8],0
0055861D   . C705 040A4303 >MOV DWORD PTR DS:[3430A04],2
00558627   . E8 8457FBFF    CALL back.0050DDB0

Save the changes, rebuild the DLL and you are ready. I am attaching here the source aswell, with only a simply MD5 encryption. Please tell me if you get the error that I am getting in my emu, if you dont, great, then I can start changing the algo. If you do get the error, help me to fix it I have no idea what is causing it...

Source download: View attachment security.rar

 

[telatoro]

Hello everyone ^_^, long time no? anyway on topic..

I would suggest someway some 'easier' method to secure it.

Make some stored procedures and/or views on database that handles the password security, hash/crypt routines with pl language, so much easier to maintenance than dlls, and just change the "query" strings on login function.

This way would be the less troublesome and still data will be stored 'secret' , client & server will not have the logic for that and also the data will travel clean. If someone get the data from database will not have something usefull (unless that whole database will be stoled, but in that case you already have the pants down by other causes).

 

[bobsobol]

Well... it's a nice idea. It has some disadvantages in that it doesn't meet the packet interception securities discussed above. But it does seem to achieve most goals in a much simpler manner. (.pl = Perl script, Yes?)

I'm always on the side of variety. It would be great to have working examples of each method, especially as it seems Sheen is almost completed already.

 

[nicktte]

Make it hash twice will never make it more "secure".
I don't see the point to create a custom algorithm. It'll be A LOT more easier for hackers.
Just use SHA-2 (256 or 512).

 

[SheenBR]

Make it hash twice will never make it more "secure".

You must be registered to see links

 

[bobsobol]

Algorithms like MD5 and SHA already use many iterations IMS, and increasing server load is not a fantastic idea. Once at either end doesn't hurt much though.

I suspect nothing greater than a 64-bit hash would fit in the packet as it stands. But the more bits you include, the lower the chances of collision. While re-hashing may not reduce security, I would think most algorithms will increase collision rate the more times they are re-hashed. (I've not studied it, or seen any study... maybe it's worth researching if you can prove the server burden of multi-hash is negligible)