From 5eea79eaf426ac3e51a09d3f3fe72c2b385abc89 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20=C4=8Cern=C3=BD?= <jcerny@redhat.com>
Date: Tue, 10 Nov 2020 11:16:00 +0100
Subject: [PATCH] Fix memory allocation

We can't assume that size of a structure is a sum of sizes of its
members because padding and alignment can be involved. In fact,
we need to allocate more bytes for the structure than the
sum of sizes of its members.

The wrong assumption caused invalid writes and invalid reads
which can be discovered by valgrind. Moreover, when run with
MALLOC_CHECK_ environment variable set to non-zero value, the
program aborted.

The memory issue happened only when NDEBUG is defined, eg. when cmake
-DCMAKE_BUILD_TYPE=RelWithDebInfo or Release, it doesn't happen if cmake
-DCMAKE_BUILD_TYPE=Debug which we usually use in Jenkins CI. This is
most likely because in debug mode the struct SEXP contains 2 additional
members which are the magic canaries and therefore is bigger.

This commit wants to fix the problem by 2 step allocation in which
first the size of the struct SEXP_val_lblk is used and then the
array of SEXPs is allocated separately.

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1891770
---
 src/OVAL/probes/SEAP/_sexp-value.h |  2 +-
 src/OVAL/probes/SEAP/sexp-value.c  | 12 ++++++------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/OVAL/probes/SEAP/_sexp-value.h b/src/OVAL/probes/SEAP/_sexp-value.h
index 426cd2c3d..e66777ef9 100644
--- a/src/OVAL/probes/SEAP/_sexp-value.h
+++ b/src/OVAL/probes/SEAP/_sexp-value.h
@@ -94,7 +94,7 @@ struct SEXP_val_lblk {
         uintptr_t nxsz;
         uint16_t  real;
         uint16_t  refs;
-        SEXP_t    memb[];
+	SEXP_t *memb;
 };
 
 size_t    SEXP_rawval_list_length (struct SEXP_val_list *list);
diff --git a/src/OVAL/probes/SEAP/sexp-value.c b/src/OVAL/probes/SEAP/sexp-value.c
index a11cbc70c..b8b3ed609 100644
--- a/src/OVAL/probes/SEAP/sexp-value.c
+++ b/src/OVAL/probes/SEAP/sexp-value.c
@@ -106,10 +106,8 @@ uintptr_t SEXP_rawval_lblk_new (uint8_t sz)
 {
         _A(sz < 16);
 
-	struct SEXP_val_lblk *lblk = oscap_aligned_malloc(
-		sizeof(uintptr_t) + (2 * sizeof(uint16_t)) + (sizeof(SEXP_t) * (1 << sz)),
-		SEXP_LBLK_ALIGN
-	);
+	struct SEXP_val_lblk *lblk = malloc(sizeof(struct SEXP_val_lblk));
+	lblk->memb = malloc(sizeof(SEXP_t) * (1 << sz));
 
         lblk->nxsz = ((uintptr_t)(NULL) & SEXP_LBLKP_MASK) | ((uintptr_t)sz & SEXP_LBLKS_MASK);
         lblk->refs = 1;
@@ -519,7 +517,8 @@ void SEXP_rawval_lblk_free (uintptr_t lblkp, void (*func) (SEXP_t *))
                         func (lblk->memb + lblk->real);
                 }
 
-		oscap_aligned_free(lblk);
+		free(lblk->memb);
+		free(lblk);
 
                 if (next != NULL)
                         SEXP_rawval_lblk_free ((uintptr_t)next, func);
@@ -540,7 +539,8 @@ void SEXP_rawval_lblk_free1 (uintptr_t lblkp, void (*func) (SEXP_t *))
                         func (lblk->memb + lblk->real);
                 }
 
-		oscap_aligned_free(lblk);
+		free(lblk->memb);
+		free(lblk);
         }
 
         return;
-- 
2.26.2