After looking through the YobGul's hashing functions I've noticed a series of security risks primarily involving buffer overflows which some of you should be concerned about who use these hashing functions (included in BnetAuth.dll).

Examine the following hashing function in YobGul's hashing algorithm:
[code]
bool _stdcall DecodeCDKey(LPCTSTR lpszCDKey, DWORD * lpdwProductId, DWORD * lpdwValue1, DWORD * lpdwValue2) {
char key[1024], value[1024];
int i, length, keylength;
bool bValid;

length = strlen(lpszCDKey);
keylength = 0;
for (i = 0; i < length; i++) {
if (isalnum(lpszCDKey[i])) {
key[keylength] = lpszCDKey[i];
keylength++;
}
}
if (keylength == 13)
bValid = DecodeStarcraftKey(key);
else if (keylength == 16)
bValid = DecodeD2Key(key);
else
return false;
strncpy(value, key, 2);
value[2] = '\0';
sscanf(value, "%X", lpdwProductId);
if (keylength == 16) {
strncpy(value, &key[2], 6);
value[6] = '\0';
sscanf(value, "%X", lpdwValue1);
strcpy(value, &key[8]);
value[8] = '\0';
sscanf(value, "%X", lpdwValue2);
}
else if (keylength == 13) {
strncpy(value, &key[2], 7);
value[7] = '\0';
sscanf(value, "%ld", lpdwValue1);
strncpy(value, &key[9], 3);
value[3] = '\0';
sscanf(value, "%ld", lpdwValue2);
}
return bValid;
}
[/code]
Now, examine more closely the following part of his DecodeCDKey() function:
[code]
length = strlen(lpszCDKey);
keylength = 0;
for (i = 0; i < length; i++) {
if (isalnum(lpszCDKey[i])) {
key[keylength] = lpszCDKey[i];
keylength++;
}
}
[/code]

This code takes the size of the CDKey and copies it into an unchecked buffer. This can cause a buffer overflow at runtime when handling the CDKey and possibly crash your program, corrupt memory, etc. and even provide a method of launching arbitrary code.

It seems however, that most bot developers have handled these necessary checks elsewhere in their code, but for those who don't you may want to heed this as a security risk in your Battle.net hashing utilization software/bots.

I would also check through his hashing functions for other buffers like those and add bound-checking.

Yeah, I noticed that awhile back. I talked to Skywing, but of course bnls doesn't use Yobgul's code (we're too good for that) so we aren't affected. But if anybody else is running a [s]bnls ripoff[/s] remote hashing server, you might want to watch out for that.

His CheckRevision also has an error. It should be GetFileTime(hFile, NULL, NULL, &ft); where GetFileTime() is called.

I think his CheckRevision also assumes there will be 3 operations in the Checksum, and if more are sent I think it will overflow.

Yes, anymore than 4 variables (including S) will cause it to overflow.

People use yobguls checkrevision? I've always found it better to call the blizzard code (and faster).

Then you're dealing with 8 files and import table patching. Though I guess you could just use one file and write the checksum to the process memory every time.

Blizzard's also leaks memory (I'm told) and it will only work in Windows computers.

[code]key[keylength] = '\0'; // Skywing Was Here - Fixed potential crash problem[/code]

There's no date on this so it's very old.
Thanks. We're safe.

[quote author=iago link=board=17;threadid=6974;start=0#msg62061 date=1085618093]
Yeah, I noticed that awhile back. I talked to Skywing, but of course bnls doesn't use Yobgul's code (we're too good for that) so we aren't affected. But if anybody else is running a [s]bnls ripoff[/s] remote hashing server, you might want to watch out for that.
[/quote]

We use Yobgul's code (very slightly modified) for SC and D2 CD-key decoding. It's nice and fast enough (there's not really that much to do...). We use our own bits of magic in War3 CD-key decoding and in CheckRevision.

Haha :)

What would we do without BNLS, Skywing, or Yoni? (Most likely use hashes but thats not the point..)

Would you spend the time to decode it? How many times must this be said? BNLS IS A SERVICE, No one is under any obligation to offer you this service, and most importantly no one is obligated to give you the hash methods

If you really wanted to, you would spend the effort to do it yourself.

[quote author=iago link=board=17;threadid=6974;start=0#msg62291 date=1085730322]it will only work in Windows computers.[/quote]Then would you mind telling me what is contained within PMACverX.mpq and XMACverX.mpq, and how does those platform's clients do the version checking?

I reversed/wrote the cdkey decoding myself in Java, and it works on the three major platforms.

[quote author=iago link=board=17;threadid=6974;start=0#msg62748 date=1085896736]
I reversed/wrote the cdkey decoding myself in Java, and it works on the three major platforms.
[/quote]

He's asking about CheckRevision, not CD-Key decoding.

[quote author=Blaze link=board=17;threadid=6974;start=0#msg62529 date=1085802979]
What would we do without BNLS, Skywing, or Yoni? (Most likely use hashes but thats not the point..)
[/quote]

Soon, I won't be using BNLS anymore :P

[quote author=Maddox link=board=17;threadid=6974;start=0#msg62831 date=1085951234]
[quote author=iago link=board=17;threadid=6974;start=0#msg62748 date=1085896736]
I reversed/wrote the cdkey decoding myself in Java, and it works on the three major platforms.
[/quote]

He's asking about CheckRevision, not CD-Key decoding.
[/quote]

Well, I did that, too, but I didn't reverse it myself.

[quote author=Inner link=board=17;threadid=6974;start=0#msg62521 date=1085801949]
Haha :)
[/quote]

?