"Should not happen" EINVAL when compiling as a Quake 3 VM with old LCC version

[TomArrow]

I understand this may be too obscure of a question for you to take time to look into, but I am trying to compile this code as a Quake 3 VM, which uses an old LCC version.

I did a bit of debug output which gives me this:

[15:54:29] bcrypt: is_little_endian 1
[15:54:29] bcrypt: is_little_endian 1
[15:54:29] bcrypt, debug vals
[15:54:29] test_key: 8b ÐÁÒÏÌØ,
[15:54:29] test_hashes[0]:i1D709vfamulimlGcq0qq3UvuUasvEa,
[15:54:29] test_hashes[1]:VUrPmXD6q/nVSSp7pNDhCR9071IfIRe,preliminary output:$2b$06$85Jfn/5QaxpFhXqxyIufgua7hp6mCreA7JKxv3SOcSiH.O08PxOae, preliminary errno: 0
[15:54:29] bcrypt: is_little_endian 1
[15:54:29] bcrypt: is_little_endian 1
[15:54:29] bcrypt: not ok, crypt_blowfish.c, line 882,
[15:54:29] p1 $2b$00$abcdefghijklmnopqrstuuVUrPmXD6q/nVSSp7pNDhCR9071IfIRe
[15:54:29] bs $2b$00$abcdefghijklmnopqrstuu
[15:54:29] p2 VUrPmXD6q/nVSSp7pNDhCR9071IfIRe
[15:54:29] th i1D709vfamulimlGcq0qq3UvuUasvEa
[15:54:29] bcrypt: not ok, crypt_blowfish.c, line 897,
[15:54:29] ae 43£ÿ£ÿÿÿ
[15:54:29] ye 43£ÿ£ÿÿÿ
[15:54:29] ai ¼YœÛp÷\z.¿- p@Óü�Çö[Ð�«�¬É�÷*“±�æ��vC sÇl™«Al9Ý�ƒ��?~¯ƒ6µà°#:äJz*ém�ÎMº43£ÿ£ÿÿÿ
[15:54:29] yi ¼YœÛp÷\z.¿- p@Óü�Çö[Ð�«�¬É�÷*“±�æ��vC sÇl™«Al9Ý�ƒ��?~¯ƒ6µà°#:äJz*ém�ÎMº
[15:54:29] bcrypt: EINVAL, crypt_blowfish.c, line 910
[15:54:29] settings: $2b$06$85Jfn/5QaxpFhXqxyIufg4
[15:54:29] Raw pw: test2, bcrypt: *0, bcrypt_errno: 22

(the line numbers wont match, as I added the debug outputs)

The bcrypt settings I'm using are: "$2b$06$85Jfn/5QaxpFhXqxyIufg4"

When I compile as a normal DLL with MSVC (the Quake 3 engine allows both DLL and Quake 3 VM as an option for modules), everything works fine. Note: The DLL in which it works fine is compiled with MSVC in Debug mode, 64 bit, v142 compiler, BF_ASM and BF_SCALE both 0.

Now the curious (or maybe not?) thing is that the hashing/derivation of the "test2" password seems to actually produce the correct result and the hash is identical to what I get with the dll. It's just the test that fails and subsequently prevents the correct result from being returned, and I'm not sure why. I read the comment mentioning some obscure GCC compiler bug, but I'm not sure this would apply.

This is the source code for the LCC compiler used from my undertanding, if it helps:
https://github.com/TomArrow/mvsdk/blob/everything/tools/lcc/lcc.c

This is the code that I use to call everything:

	int clientNum = -1;
	char pw[64];
	const static char settings[64] = BCRYPT_SETTINGS;
	char output[64];

	if (trap_Argc() < 3) {
		CG_Printf("usage /login <username> <password>\n");
		return;
	}

	pw[0] = '\0';

	trap_Argv(2,pw,sizeof(pw));

	_crypt_blowfish_rn(pw, settings, output, 64);

I am including crypt_blowfish.h. I don't really need the gensalt functionality in my use case (I just use one salt for the entire program), so I only included crypt_blowfish.h and crypt_blowfish.c. I made sure that BF_ASM and BF_SCALE are both 0.

Not sure if it matters but I know that this QVM system is limited to 32 bit integers (and pointers) and it doesn't deal well with signed shorts, though the latter just produces a compiler error, so I don't think that's related.

Maybe I just forgot to initialize some global variables or something, no idea. Any help or pointer would be appreciated if you can spare the time.

[TomArrow]

Hmm judging by the debug outputs, the calculated hash is identical to test_hashes[1] but it's being compared against test_hashes[0].

[solardiz]

Looks like the failure has to do with processing of 8-bit characters. Maybe the compiler or VM has issues with handling of signed vs. unsigned char, and in particular with sign/zero extension. Or maybe the hash prefix check (2b vs. other flavors) or the tests get miscompiled.

I suggest you test with your own input password containing an 8-bit character, and see whether you get the right hash or not.

[solardiz]

One specific guess is that the cast on this line might be non-working:

                        tmp[0] |= (unsigned char)*ptr; /* correct */

You can try writing it differently, e.g.:

                        tmp[0] |= *(unsigned char *)ptr; /* correct */

or:

                        tmp[0] |= (BF_word)(unsigned char)*ptr; /* correct */

[TomArrow]

Seems like

 tmp[0] |= *(unsigned char *)ptr; /* correct */

did the trick.

tmp[0] |= (BF_word)(unsigned char)*ptr; /* correct */

on the other hand didn't work, unless I made some mistake.

I'll do more tests tomorrow but thanks a lot already!

[solardiz]

Happy to hear we found a workaround @TomArrow!

This sounds like an LCC bug. Is there an upstream for LCC where you could check whether the bug is still present, and report the bug if needed? Could also update LCC in or report to other projects embedding LCC, such as the SDK you're using.

[TomArrow]

Looks like LCC wasn't actually updated for 10 years, but it seems like Issues are still getting responses, so I created an issue there. This is all very much out of my depth but maybe your findings could help describe the issue, so I linked this issue as well.

I've tested some more strings today including ones with some values between ~161 and 255. Seems to be stable with your fix. Thanks again!

[TomArrow]

Actually now I've been told it may not be quite the same LCC version. But maybe it's still relevant, so I'll keep that issue open.

[solardiz]

Thanks. I suggest you edit the LCC issue's title to "(unsigned char) cast from char ignored".

[TomArrow]

Thanks. I suggest you edit the LCC issue's title to "(unsigned char) cast from char ignored".

Done.