#11 Fix stack overflow in rpm2extents
Merged 2 years ago by ngompa. Opened 2 years ago by richardphibel.

Fix stack overflow in rpm2extents
Richard Phibel • 2 years ago  
@@ -0,0 +1,80 @@ 

+ From 937f9bc67b905851c78719d8397926eaa97b174a Mon Sep 17 00:00:00 2001

+ From: Richard Phibel <richardphibel@meta.com>

+ Date: Mon, 22 May 2023 05:16:51 +0200

+ Subject: [PATCH] Fix stack overflow

+ 

+ Creation of array struct digestoffset offsets[rpmfiFC(fi)] caused a

+ stack overflow because the total size is greater than 8M which is the

+ stack size limit on Linux. To fix the issue, the array is allocated on

+ the heap.

+ 

+ I used AddressSanitizer to find the root cause of the issue. It found a

+ number of memory leaks so I fixed them as well.

+ ---

+  rpm2extents.c | 15 +++++++++++----

+  1 file changed, 11 insertions(+), 4 deletions(-)

+ 

+ diff --git a/rpm2extents.c b/rpm2extents.c

+ index c2a373914..0ee8666fa 100644

+ --- a/rpm2extents.c

+ +++ b/rpm2extents.c

+ @@ -226,6 +226,7 @@ exit:

+      if(msg) {

+  	free(msg);

+      }

+ +    rpmtsFree(ts);

+      return rc;

+  }

+  

+ @@ -243,6 +244,7 @@ static void sanitizeSignatureHeader(Header * sigh)

+  	*sigh = headerLink(nh);

+  	headerFree(nh);

+      }

+ +    rpmtdFreeData(&td);

+  }

+  

+  static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)

+ @@ -281,6 +283,8 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)

+      rpmfiles files = NULL;

+      rpmfi fi = NULL;

+      char *msg = NULL;

+ +    struct digestoffset *offsets = NULL;

+ +    digestSet ds = NULL;

+  

+      fdo = fdDup(STDOUT_FILENO);

+  

+ @@ -357,10 +361,8 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)

+  	 * now?)

+  	 */

+  	diglen = (uint32_t) rpmDigestLength(rpmfiDigestAlgo(fi));

+ -	digestSet ds =

+ -	    digestSetCreate(rpmfiFC(fi), digestSetHash, digestSetCmp,

+ -			    NULL);

+ -	struct digestoffset offsets[rpmfiFC(fi)];

+ +	ds = digestSetCreate(rpmfiFC(fi), digestSetHash, digestSetCmp, NULL);

+ +	offsets = xcalloc(rpmfiFC(fi), sizeof(*offsets));

+  	pos = RPMLEAD_SIZE + headerSizeof(sigh, HEADER_MAGIC_YES);

+  

+  	/* main headers are aligned to 8 byte boundry */

+ @@ -494,6 +496,10 @@ static rpmRC process_package(FD_t fdi, FD_t digestori, FD_t validationi)

+      rpmfilesFree(files);

+      rpmfiFree(fi);

+      headerFree(h);

+ +    headerFree(sigh);

+ +    free(offsets);

+ +    Fclose(fdo);

+ +    digestSetFree(ds);

+      return rc;

+  }

+  

+ @@ -693,6 +699,7 @@ int main(int argc, char *argv[]) {

+  

+      FD_t fdi = fdDup(STDIN_FILENO);

+      rc = teeRpm(fdi, algos, nb_algos);

+ +    Fclose(fdi);

+      if (rc != RPMRC_OK) {

+  	/* translate rpmRC into generic failure return code. */

+  	return EXIT_FAILURE;

+ -- 

+ 2.40.1

+ 

file modified
+5 -1
@@ -42,7 +42,7 @@ 

  

  %global rpmver 4.16.1.3

  #global snapver rc1

- %global rel 22.3

+ %global rel 22.4

  %global sover 9

  

  %global srcver %{rpmver}%{?snapver:-%{snapver}}
@@ -190,6 +190,7 @@ 

  Patch9930: 0030-rpmcow-Make-rpm-i-install-package-without-the-need-o.patch

  Patch9931: 0031-rpmcow-denylist.patch

  Patch9932: 0032-rpmcow-workaround.patch

+ Patch9933: 0033-rpmcow-fix-stack-overflow-in-rpm2extents.patch

  Provides: rpm(pr1470)

  Provides: rpm(pr1470_1)

  
@@ -775,6 +776,9 @@ 

  %doc doc/librpm/html/*

  

  %changelog

+ * Mon May 22 2023 Richard Phibel <richardphibel@meta.com> - 4.16.1.3-22.4

+ - Fix stack overflow in rpm2extents and various memory leaks

+ 

  * Sat Feb 11 2023 Davide Cavalca <dcavalca@centosproject.org> - 4.16.1.3-22.3

  - Drop our selinux policy as it's been subsumed by the main one

  

This fixes an issue encountered for large RPMs.

Creation of array struct digestoffset offsets[rpmfiFC(fi)] causes a stack overflow because, for a large RPM, the total size of the array is greater than 8M which is the
stack size limit on Linux. To fix the issue, the array is now allocated on the heap.

I used AddressSanitizer to find the root cause of the issue. It found a number of memory leaks so I fixed them as well.

Pull-Request has been merged by ngompa

2 years ago