Radish alpha
H
rad:z3QDZAW2FAfuLvihrhiyDC9fAD8G9
HardenedBSD Package Manager
Radicle
Git
external: import libder changes to zero out payloads
Kyle Evans committed 2 years ago
commit df217a936f62b6971c9b35da693224489963247d
parent 26f1275
5 files changed +97 -9
modified external/libder/README.md
@@ -8,6 +8,11 @@ re-encode the resulting tree would apply any normalization expected by a DER
decoder.  The author's use is primarily to decode/encode ECC keys for
interoperability with OpenSSL.

+
The authoritative source for this software is located at
+
https://git.kevans.dev/kevans/libder, but it's additionally mirrored to
+
[GitHub](https://github.com/kevans91/libder) for user-facing interactions.
+
Pull requests and issues are open on GitHub.
+

## What is libder not?

libder is not intended to be a general-purpose library for working with DER/BER
modified external/libder/libder/libder_obj.c
@@ -74,7 +74,11 @@ libder_obj_alloc(struct libder_ctx *ctx, struct libder_tag *type,

	obj = libder_obj_alloc_internal(ctx, type, payload, length, 0);
	if (obj == NULL) {
-
		free(payload);
+
		if (length != 0) {
+
			libder_bzero(payload, length);
+
			free(payload);
+
		}
+

		libder_set_error(ctx, LDE_NOMEM);
	}

@@ -102,7 +106,11 @@ libder_obj_alloc_simple(struct libder_ctx *ctx, uint8_t stype,

	obj = libder_obj_alloc_internal(ctx, type, payload, length, LDO_OWNTAG);
	if (obj == NULL) {
-
		free(payload);
+
		if (length != 0) {
+
			libder_bzero(payload, length);
+
			free(payload);
+
		}
+

		libder_type_free(type);
		libder_set_error(ctx, LDE_NOMEM);
	}
@@ -241,7 +249,11 @@ libder_obj_free(struct libder_object *obj)
	DER_FOREACH_CHILD_SAFE(child, obj, tmp)
		libder_obj_free(child);

-
	free(obj->payload);
+
	if (obj->payload != NULL) {
+
		libder_bzero(obj->payload, obj->length);
+
		free(obj->payload);
+
	}
+

	libder_type_free(obj->type);
	free(obj);
}
@@ -817,12 +829,20 @@ violated:
	obj->children = NULL;

	if (strict_violation) {
-
		free(coalesced_data);
+
		if (coalesced_data != NULL) {
+
			libder_bzero(coalesced_data, offset);
+
			free(coalesced_data);
+
		}
+

		return (false);
	}

	/* Finally, swap out the payload. */
-
	free(obj->payload);
+
	if (obj->payload != NULL) {
+
		libder_bzero(obj->payload, obj->length);
+
		free(obj->payload);
+
	}
+

	obj->length = offset;
	obj->payload = coalesced_data;
	obj->type->tag_constructed = false;
modified external/libder/libder/libder_private.h
@@ -11,7 +11,12 @@
#include <assert.h>
#include <signal.h>
#include <stdbool.h>
-

+
#ifdef __APPLE__
+
#define	__STDC_WANT_LIB_EXT1__	1
+
#include <string.h>	/* memset_s */
+
#else
+
#include <strings.h>	/* explicit_bzero */
+
#endif
#include "libder.h"

/* FreeBSD's sys/cdefs.h */
@@ -139,6 +144,17 @@ libder_type_simple(const struct libder_tag *type)
	return (encoded);
}

+
static inline void
+
libder_bzero(uint8_t *buf, size_t bufsz)
+
{
+

+
#ifdef __APPLE__
+
	memset_s(buf, bufsz, 0, bufsz);
+
#else
+
	explicit_bzero(buf, bufsz);
+
#endif
+
}
+

size_t	 libder_get_buffer_size(struct libder_ctx *);
void	 libder_set_error(struct libder_ctx *, int, const char *, int);

modified external/libder/libder/libder_read.c
@@ -83,7 +83,10 @@ payload_free(struct libder_payload *payload)
	if (!payload->payload_heap)
		return;

-
	free(payload->payload_data);
+
	if (payload->payload_data != NULL) {
+
		libder_bzero(payload->payload_data, payload->payload_size);
+
		free(payload->payload_data);
+
	}

	payload->payload_heap = false;
	payload->payload_data = NULL;
@@ -126,7 +129,10 @@ libder_stream_init(struct libder_ctx *ctx, struct libder_stream *stream)
static void
libder_stream_free(struct libder_stream *stream)
{
-
	free(stream->stream_buf);
+
	if (stream->stream_buf != NULL) {
+
		libder_bzero(stream->stream_buf, stream->stream_bufsz);
+
		free(stream->stream_buf);
+
	}
}

static void
@@ -325,6 +331,36 @@ libder_stream_refill(struct libder_stream *stream, size_t req)
	return (&stream->stream_buf[offset]);
}

+
/*
+
 * We can't just use realloc() because it won't provide any guarantees about
+
 * the previous region if it can't just resize in-place, so we'll always just
+
 * allocate a new one and copy ourselves.
+
 */
+
static uint8_t *
+
libder_read_realloc(uint8_t *ptr, size_t oldsz, size_t newsz)
+
{
+
	uint8_t *newbuf;
+

+
	if (oldsz == 0)
+
		assert(ptr == NULL);
+
	else
+
		assert(ptr != NULL);
+
	assert(newsz > oldsz);
+

+
	newbuf = malloc(newsz);
+
	if (newbuf == NULL)
+
		return (NULL);
+

+
	if (oldsz != 0) {
+
		memcpy(newbuf, ptr, oldsz);
+

+
		libder_bzero(ptr, oldsz);
+
		free(ptr);
+
	}
+

+
	return (newbuf);
+
}
+

#define	BER_TYPE_LONG_BATCH	0x04

static bool
@@ -487,6 +523,13 @@ der_read_structure(struct libder_ctx *ctx, struct libder_stream *stream,
		} else {
			uint8_t *payload_data;

+
			/*
+
			 * We play it conservative here: we could allocate the
+
			 * buffer up-front, but we have no idea how much data we
+
			 * actually have to receive!  The length is a potentially
+
			 * attacker-controlled aspect, so we're cautiously optimistic
+
			 * that it's accurate.
+
			 */
			payload_data = NULL;

			offset = 0;
@@ -497,14 +540,17 @@ der_read_structure(struct libder_ctx *ctx, struct libder_stream *stream,

				req = MIN(stream->stream_bufsz, resid);
				if ((buf = libder_stream_refill(stream, req)) == NULL) {
+
					libder_bzero(payload_data, offset);
					free(payload_data);

					libder_set_error(ctx, LDE_SHORTDATA);
					goto failed;
				}

-
				next_data = realloc(payload_data, offset + req);
+
				next_data = libder_read_realloc(payload_data,
+
				    offset, offset + req);
				if (next_data == NULL) {
+
					libder_bzero(payload_data, offset);
					free(payload_data);

					libder_set_error(ctx, LDE_NOMEM);
modified external/libder/libder/libder_write.c
@@ -213,6 +213,7 @@ libder_write(struct libder_ctx *ctx, struct libder_object *root, uint8_t *buf,
	mwrite.buf = buf;
	mwrite.bufsz = *bufsz;
	if (!libder_write_object(ctx, root, &memory_write, &mwrite)) {
+
		libder_bzero(mwrite.buf, mwrite.offset);
		free(buf);
		return (NULL);	/* XXX Error */
	}