Missing Check for file bad size

[ManSoSec]

src/ssl.c

There is following check in this function: wolfSSL_CertManagerVerify
after sz = XFTELL(file); XREWIND(file);:

if (sz > MAX_WOLFSSL_FILE_SIZE || sz <= 0) {
        WOLFSSL_MSG("CertManagerVerify file bad size");
        XFCLOSE(file);
        return WOLFSSL_BAD_FILE;
    }

While it is missed in wolfSSL_SetTmpDH_file_wrapper and ProcessFile

[dgarske]

Hi ManSoSec,

Thanks for pointing out that we are not consistent with the enforcement of MAX_WOLFSSL_FILE_SIZE. We will review and let you know the results.

Thanks,
David Garske, wolfSSL

[ManSoSec]

ProcessFile function is in the same situation: missing the enforcement of MAX_WOLFSSL_FILE_SIZE

[ManSoSec]

Hi @dgarske,

Is there any update on this issue? Thanks!

[dgarske]

HI @ManSoSec : Thanks for checking in. It's on my list, but low priority. Is this causing you guys any issues?

Thanks,
David Garske, wolfSSL

[ManSoSec]

Hi @dgarske, Thanks for your reply. Our research focuses on finding bugs and one of the tools we are testing is wolfSSL. We just wanted to be sure if this reported bug is a valid one. Feedbacks from the developers can help both parties to pursue more reliable research as well as fixing more valid bugs. Thanks!

[ManSoSec]

Hi @dgarske

May I ask what is the potential security consequence of the 14 missing checks? I feel one can be memory leak as big files can be sent to WolfSSL and XMALLOC
allocate memory to them so wasting the memory. Thanks!

[dgarske]

Hi @ManSoSec ,

Again thank for your reporting this inconsistency. The PR #2598 just adds sanity checks for file sizes. In my opinion there is no security risk here. Someone would to have already compromised access to the file system and access the certificates being loaded. Then would have to enlarge the size of a file and load it with valid content. This would cause additional memory use, but not leaks.

Thanks,
David Garske, wolfSSL

[ManSoSec]

Hi @dgarske

Sure! Thank you for the clarification!