[Crash] RAND_load_file() with specific file-sizes

[gvanem]

Some file-sizes given to this command:

openssl.exe rand -rand rand-file -hex 10

crashes in some mysterious ways while freeing the associated RAND_POOL. The call-stack from WinDbg:

ntdll!RtlReportCriticalFailure+0x88
ntdll!RtlpReportHeapFailure+0x2f
ntdll!RtlpHpHeapHandleError+0x6e
ntdll!RtlpLogHeapFailure+0x41
ntdll!RtlFreeHeap+0x4cbbc
ucrtbase!_free_base+0x1b
ucrtbase!free+0x18
libcrypto_1_1!CRYPTO_free(void * str = 0x011d0000, char * file = 0x6b77f9c4 "crypto/rand/rand_lib.c", int line = 0n506)+0x45 [f:\mingw32\src\inet\crypto\openssl\crypto\mem.c @ 307] 
libcrypto_1_1!rand_pool_free(struct rand_pool_st * pool = 0x075a84b0)+0x36 [f:\mingw32\src\inet\crypto\openssl\crypto\rand\rand_lib.c @ 506] 
libcrypto_1_1!rand_drbg_restart(struct rand_drbg_st * drbg = 0x0804daf8, unsigned char * buffer = 0x010fe4f8 "", unsigned int len = 0x16, unsigned int entropy = 0xb0)+0x1cd [f:\mingw32\src\inet\crypto\openssl\crypto\rand\drbg_lib.c @ 632] 
libcrypto_1_1!drbg_add(void * buf = 0x010fe4f8, int num = 0n22, double randomness = 22)+0x8b [f:\mingw32\src\inet\crypto\openssl\crypto\rand\drbg_lib.c @ 1062] 
libcrypto_1_1!RAND_add(void * buf = 0x010fe4f8, int num = 0n22, double randomness = 22)+0x2b [f:\mingw32\src\inet\crypto\openssl\crypto\rand\rand_lib.c @ 783] 
libcrypto_1_1!RAND_load_file(char * file = 0x076233c1 "rand-file", long bytes = 0n-1)+0x186 [f:\mingw32\src\inet\crypto\openssl\crypto\rand\randfile.c @ 142] 
openssl!loadfiles+0x2b [f:\mingw32\src\inet\crypto\openssl\apps\app_rand.c @ 46] 
openssl!opt_rand(int opt = 0n1501)+0x7b [f:\mingw32\src\inet\crypto\openssl\apps\app_rand.c @ 85] 
openssl!rand_main(int argc = 0n5, char ** argv = 0x0762338c)+0xb3 [f:\mingw32\src\inet\crypto\openssl\apps\rand.c @ 68] 
openssl!do_cmd(struct lhash_st_FUNCTION * prog = 0x07624cc0, int argc = 0n5, char ** argv = 0x0762338c)+0x68 [f:\mingw32\src\inet\crypto\openssl\apps\openssl.c @ 620] 
openssl!main(int argc = 0n0, char ** argv = 0x011e45b8)+0x24d 

I created the rand-file using CygWin's truncate:

Command	Result
truncate.exe -s 10000000 rand-file	OKAY
truncate.exe -s 10000 rand-file	OKAY
truncate.exe -s 1000 rand-file	OKAY
truncate.exe -s 1046 rand-file	not OKAY
truncate.exe -s 2048 rand-file	OKAY

I'm on Windows-10 using MSVC-2017.
Some other discussion of this issue is here too:
3064b55#commitcomment-30980301

[mspncp]

I can now reproduce the problem with Visual Studio on Windows.

[mspncp]

This should fix your crash, but it's only half of the problem. I will open a pull request, but it will take little while because I have to leave now.

diff --git a/crypto/rand/rand_lib.c b/crypto/rand/rand_lib.c
index f40c513ce8..52e2621948 100644
--- a/crypto/rand/rand_lib.c
+++ b/crypto/rand/rand_lib.c
@@ -200,6 +200,9 @@ size_t rand_drbg_get_entropy(RAND_DRBG *drbg,
     }

  err:
+    if (ret == 0 && drbg->pool != NULL)
+        drbg->pool = NULL;
+
     rand_pool_free(pool);
     return ret;
 }

[gvanem]

That fix works fine (no crashes with any sizes I've tried). Why is it only half of the problem?

[mspncp]

Actually the first patch was only part one of three:

• Part one fixes the memory corruption after the reseeding failed
• Part two adds an error message (see patch below)
• Part three fixes the true problem: RAND_add() fails if the buffer size is less than 32 (work in progress...)

More details in the upcoming pull request. Please be patient, I'm a hobby committer... ;-)

For the moment, I would ask you to apply this second patch and try whether openssl rand now returns a useful error message. You should get it whenever the randfile size is k*1024 + r with r<32, in particular for all sizes < 32.

commit bafbab69b7cb43a0e13feeded86f47e703a40ab8 (HEAD -> master)
Author: Dr. Matthias St. Pierre <Matthias.St.Pierre@ncp-e.com>
Date:   Sat Oct 20 16:53:57 2018 +0200

    RAND_load_file(): return error if reseeding failed

diff --git a/crypto/rand/randfile.c b/crypto/rand/randfile.c
index 89720eb5cf..028c1281c0 100644
--- a/crypto/rand/randfile.c
+++ b/crypto/rand/randfile.c
@@ -148,6 +148,12 @@ int RAND_load_file(const char *file, long bytes)

     OPENSSL_cleanse(buf, sizeof(buf));
     fclose(in);
+    if (!RAND_status()) {
+        RANDerr(RAND_F_RAND_LOAD_FILE, RAND_R_RESEED_ERROR);
+        ERR_add_error_data(2, "Filename=", file);
+        return -1;
+    }
+
     return ret;
 }

[mspncp]

Note: although the seeding fails in your case, the DRBG will automatically recover from the error in the next RAND_bytes() call. So the output of openssl rand can be trusted. But of course the failure needs to be fixed.

[gvanem]

For the moment, I would ask you to apply this second patch and try whether openssl rand now
returns a useful error message.

After applying all your patches and trying with a

truncate -s 2055 rand-file
openssl rand rand-file -hex 10

gives:

Can't load rand-file into RNG
105724:error:2407307D:random number generator:rand_pool_bytes_needed:random pool overflow:crypto/rand/rand_lib.c:609:
105724:error:2406E06E:random number generator:RAND_DRBG_reseed:error retrieving entropy:crypto/rand/drbg_lib.c:508:
105724:error:2406F076:random number generator:RAND_load_file:reseed error:crypto/rand/randfile.c:157:Filename=rand-file

No crash anymore, but I've no idea why the size should matter!?

[mspncp]

I've no idea why the size should matter!?

Because it's a bug which I introduced in 3064b55 :-)

[mspncp]

(Hint: 32 bytes corresponds to 256 bits which is the security strength of the DRBG.)

[mspncp]

See #7455 for the immediate fix and #7456 for the full story...

[gvanem]

Good job. Feel free to close this issue when you wish.

[mspncp]

Thank you very much for your appreciation. But actually I can return the compliment because your report pointed me to a whole bunch of minor problems.

Don't worry about the issue, it will be closed automatically when the PR is merged.

Regards,
Matthias

[mspncp]

Okay... the notification page tells me that the closing hook was issued, but it's still open. Probably because GitHub is still in confusion... Well, then I'll close it myself.