[PATCH] libelf: Free and clear rawdata_base and zdata_base consistently

[PATCH] libelf: Free and clear rawdata_base and zdata_base consistently

[Mark Wielaard]

There could be a leak if a program called elf_strptr on a compressed
section, but the program never requests the (uncompressed) section data,
but does explicitly (re)compress that same section data.

Fix this by explicitly always freeing and clearing the zdata_base
and rawdata_base in __libelf_reset_rawdata and elf_compress. Also
clear zdata_base in elf_end so the pointer isn't indeterminate when
it is being used in a later comparison against rawdata_base.

	* libelf/elf_compress.c (elf_compress): Explicitly free
	zdata_base before clearing.
	(__libelf_reset_rawdata): Free zdata_base if it isn't
	(going to be) used for rawdata_base. Explicitly clear
	rawdata_base and zdata_base after free.
	* libelf/elf_end.c (elf_end): Clear zdata_base after free.

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 libelf/elf_compress.c | 14 +++++++++++++-
 libelf/elf_end.c      |  5 ++++-
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/libelf/elf_compress.c b/libelf/elf_compress.c
index f13b41ba..c7283c6a 100644
--- a/libelf/elf_compress.c
+++ b/libelf/elf_compress.c
@@ -1,5 +1,6 @@
 /* Compress or decompress a section.
    Copyright (C) 2015, 2016 Red Hat, Inc.
+   Copyright (C) 2023, Mark J. Wielaard <mark@klomp.org>
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -523,10 +524,20 @@ __libelf_reset_rawdata (Elf_Scn *scn, void *buf, size_t size, size_t align,
   if (scn->data_base != scn->rawdata_base)
     free (scn->data_base);
   scn->data_base = NULL;
+  if (scn->zdata_base != buf
+      && scn->zdata_base != scn->rawdata_base)
+    {
+      free (scn->zdata_base);
+      scn->zdata_base = NULL;
+    }
   if (scn->elf->map_address == NULL
       || scn->rawdata_base == scn->zdata_base
       || (scn->flags & ELF_F_MALLOCED) != 0)
-    free (scn->rawdata_base);
+    {
+      free (scn->rawdata_base);
+      scn->rawdata_base = NULL;
+      scn->zdata_base = NULL;
+    }
 
   scn->rawdata_base = buf;
   scn->flags |= ELF_F_MALLOCED;
@@ -677,6 +688,7 @@ elf_compress (Elf_Scn *scn, int type, unsigned int flags)
 	 data around, but since that might have been multiple Elf_Data
 	 buffers let the user uncompress it explicitly again if they
 	 want it to simplify bookkeeping.  */
+      free (scn->zdata_base);
       scn->zdata_base = NULL;
 
       return 1;
diff --git a/libelf/elf_end.c b/libelf/elf_end.c
index 3e5d4c86..89727cb3 100644
--- a/libelf/elf_end.c
+++ b/libelf/elf_end.c
@@ -157,7 +157,10 @@ elf_end (Elf *elf)
 		   rawdata_base.  If it is already used it will be
 		   freed below.  */
 		if (scn->zdata_base != scn->rawdata_base)
-		  free (scn->zdata_base);
+		  {
+		    free (scn->zdata_base);
+		    scn->zdata_base = NULL;
+		  }
 
 		/* If the file has the same byte order and the
 		   architecture doesn't require overly stringent
-- 
2.39.3

Re: [PATCH] libelf: Free and clear rawdata_base and zdata_base consistently

[Mark Wielaard]

Hi,

On Sun, Jul 30, 2023 at 05:44:20PM +0200, Mark Wielaard wrote:
> There could be a leak if a program called elf_strptr on a compressed
> section, but the program never requests the (uncompressed) section data,
> but does explicitly (re)compress that same section data.
> 
> Fix this by explicitly always freeing and clearing the zdata_base
> and rawdata_base in __libelf_reset_rawdata and elf_compress. Also
> clear zdata_base in elf_end so the pointer isn't indeterminate when
> it is being used in a later comparison against rawdata_base.
> 
> 	* libelf/elf_compress.c (elf_compress): Explicitly free
> 	zdata_base before clearing.
> 	(__libelf_reset_rawdata): Free zdata_base if it isn't
> 	(going to be) used for rawdata_base. Explicitly clear
> 	rawdata_base and zdata_base after free.
> 	* libelf/elf_end.c (elf_end): Clear zdata_base after free.

Pushed.

Mark