libressl / portable

LibreSSL Portable itself. This includes the build scaffold and compatibility layer that builds portable LibreSSL from the OpenBSD source code. Pull requests or patches sent to tech@openbsd.org are welcome.
https://www.libressl.org
1.35k stars 269 forks source link

Possible memory leak on EVP_DigestInit_ex() - FreeBSD #1098

Closed devnull-hub-lab closed 1 week ago

devnull-hub-lab commented 1 week ago

Hey folks,

I'm using FreeBSD 14.1 (tested on 14.0 too) + CLANG, and I think there is a memory leak on EVP_DigestInit_ex().

When compiled with -fsanitize=address flag, EVP_DigestInit_ex() always return some error. Without -fsanitize=address flag, it works perfectly.

I wrote a simple partial C code. During debug, without ASan, method EVP_DigestInit_ex() pass successfully. With -fsanitize=address flag, the condition EVP_DigestInit_ex() didn't pass.

#include <stdio.h>
#include <openssl/evp.h>
#include <string.h>

int main() {
    const char *message = "My Message to be hashed";
    unsigned char digest[EVP_MAX_MD_SIZE];
    unsigned int digest_len;

    EVP_MD_CTX *mdctx;
    if ((mdctx = EVP_MD_CTX_new()) == NULL)
    {
        fprintf(stderr, "Error EVP_MD_CTX_new \n");
        return 1;
    }

    if (!EVP_DigestInit_ex(mdctx, EVP_sha256(), NULL))
    {
        fprintf(stderr, "Error EVP_DigestInit_ex \n");
        EVP_MD_CTX_free(mdctx);
        //abort();
        return 1;
    }

    //.....I ignored EVP_DigestUpdate(), EVP_DigestFinal_ex() in this PoC, since the error occurred in EVP_DigestInit_ex() above.

    EVP_MD_CTX_free(mdctx);

    return 0;
}

On GNU/Linux (somehow), it always works, with or without the address sanitizer flag.

I tested on OpenSSL too (default on FreeBSD basesystem) and I got same problem. I also tested without ASLR security restrictions (kern.elf64.aslr.pie_enable=0 and kern.elf64.aslr.enable=0)

Is it possible that it's a memory leak, or some quirk that needs to be handled differently?

Thanks,

botovq commented 1 week ago

I don't think there's a leak in EVP_DigestInit_ex(), definitely not with your usage example. The only place where your test program's call to EVP_DigestInit_ex() can fail is the return 0; in evp_digest.c line 150. In order to reach this, FreeBSD's -fsanitize=address must be doing something strange and return a NULL pointer. No code path here should be FreeBSD-specific.

I would suggest you compile with debugging symbols and step through the code with the debugger to see what fails and why.


Let's look at it in detail:

    if ((mdctx = EVP_MD_CTX_new()) == NULL)

This is a simple wrapper around calloc(3). In particular, mdctx will point at a chunk of zeroed memory on success.

    if (!EVP_DigestInit_ex(mdctx, EVP_sha256(), NULL))

EVP_sha256() returns a static pointer, which is in particular never NULL.

131 int
132 EVP_DigestInit_ex(EVP_MD_CTX *ctx, const EVP_MD *type, ENGINE *impl)
133 {
134         EVP_MD_CTX_clear_flags(ctx, EVP_MD_CTX_FLAG_CLEANED);

ctx is a pointer to zeroed memory. The above clears EVP_MD_CTX_FLAG_CLEANED so ctx is still a pointer to zeroed memory.

135
136         if (ctx->digest != type) {

since ctx->digest == NULL is different from type == &md_sha256, we enter here.

137                 if (ctx->digest && ctx->digest->ctx_size && ctx->md_data &&
138                     !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE)) {
139                         freezero(ctx->md_data, ctx->digest->ctx_size);
140                         ctx->md_data = NULL;
141                 }

ctx->digest == NULL, so we don't enter the body of the if block and jump here.

142                 ctx->digest = type;

ctx->digest is set to &sha_md. No leak here since it was NULL.

143                 if (!(ctx->flags & EVP_MD_CTX_FLAG_NO_INIT) && type->ctx_size) {

we enter here since the flag EVP_MD_CTX_FLAG_NO_INIT isn't set, and since sha_md->ctx_size == sizeof(EVP_MD *) + sizeof(SHA256_CTX) is not zero.

144                         ctx->update = type->update;

set the update() handler (which was NULL previously).

145                         ctx->md_data = calloc(1, type->ctx_size);

allocate ctx->md_data. No leak since ctx->md_data was NULL.

146                         if (ctx->md_data == NULL) {
147                                 EVP_PKEY_CTX_free(ctx->pctx);
148                                 ctx->pctx = NULL;
149                                 EVPerror(ERR_R_MALLOC_FAILURE);
150                                 return 0;

As noted above, this is the only place where we can fail.

151                         }
152                 }
153         }
154         if (ctx->pctx) {

There's no pctx set, so we don't enter here.

155                 int r;
156                 r = EVP_PKEY_CTX_ctrl(ctx->pctx, -1, EVP_PKEY_OP_TYPE_SIG,
157                     EVP_PKEY_CTRL_DIGESTINIT, 0, ctx);
158                 if (r <= 0 && (r != -2))
159                         return 0;
160         }

No flags are set, so we don't return early below

161         if (ctx->flags & EVP_MD_CTX_FLAG_NO_INIT)
162                 return 1;
163         return ctx->digest->init(ctx);
164  }

init() is sha256_init() which wraps SHA256_Init() which can't fail.

botovq commented 1 week ago

I'm pretty sure this is due to FreeBSD and NetBSD's libasan intercepting SHA256_Init because they set SANITIZER_INTERCEPT_SHA2, so ASAN replaces it with one of incompatible signature because this interception is designed for the libc (or libmd) versions, which don't return anything.

Your test program's EVP_DigestInit_ex() will also fail with EVP_sha512(), but it will succeed with EVP_sha3_256() or EVP_sha1() (only on FreeBSD, not NetBSD because it intercepts SHA-1 as well) for example.

This is an issue that needs to be solved by FreeBSD and NetBSD. Not our problem, so I'm going to close this.

botovq commented 1 week ago

Reported to FreeBSD: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=281685

devnull-hub-lab commented 1 week ago

Thanks @botovq for further investigation!