File sysstat-12.0.2-CVE-2018-19416-and-CVE-2018-1951... of Package sysstat

Overview Repositories Revisions Requests Users Attributes Meta

File sysstat-12.0.2-CVE-2018-19416-and-CVE-2018-19517.patch of Package sysstat (Revision 2c5ba3b60b9ceeff3ef79f99f6b52bcd)

Currently displaying revision 2c5ba3b60b9ceeff3ef79f99f6b52bcd , Show latest

From fbc691eaaa10d0bcea6741d5a223dc3906106548 Mon Sep 17 00:00:00 2001
From: Sebastien GODARD <sysstat@users.noreply.github.com>
Date: Mon, 26 Nov 2018 14:32:05 +0100
Subject: [PATCH] Fix #196 and #199: Out of bound reads security issues

Check args before calling memmove() and memset() in remap_struct()
function to avoid out of bound reads which would possibly lead to
unknown code execution and/or sadf command crash.

Signed-off-by: Sebastien GODARD <sysstat@users.noreply.github.com>
---
 sa.h        |  5 ++--
 sa_common.c | 66 ++++++++++++++++++++++++++++++++++++-----------------
 sadf.c      |  2 +-
 sar.c       | 13 ++++++-----
 4 files changed, 56 insertions(+), 30 deletions(-)

Index: sysstat-12.0.2/sa.h
===================================================================
--- sysstat-12.0.2.orig/sa.h
+++ sysstat-12.0.2/sa.h
@@ -1355,11 +1355,11 @@ void read_file_stat_bunch
 __nr_t read_nr_value
 	(int, char *, struct file_magic *, int, int, int);
 int read_record_hdr
-	(int, void *, struct record_header *, struct file_header *, int, int);
+        (int, void *, struct record_header *, struct file_header *, int, int, size_t);
 void reallocate_all_buffers
 	(struct activity *, __nr_t);
 void remap_struct
-	(unsigned int [], unsigned int [], void *, unsigned int, unsigned int);
+        (unsigned int [], unsigned int [], void *, unsigned int, unsigned int, size_t);
 void replace_nonprintable_char
 	(int, char *);
 int sa_fread
Index: sysstat-12.0.2/sa_common.c
===================================================================
--- sysstat-12.0.2.orig/sa_common.c
+++ sysstat-12.0.2/sa_common.c
@@ -1280,12 +1280,14 @@ void swap_struct(unsigned int types_nr[]
  * @f_size	Size of the structure containing statistics. This is the
  *		size of the structure *read from file*.
  * @g_size	Size of the structure expected by current sysstat version.
+ * @b_size	Size of the buffer pointed by @ps.
  ***************************************************************************
  */
 void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[],
-		  void *ps, unsigned int f_size, unsigned int g_size)
+		  void *ps, unsigned int f_size, unsigned int g_size, size_t b_size)
 {
 	int d;
+	size_t n;
 
 	/* Sanity check */
 	if (MAP_SIZE(ftypes_nr) > f_size)
@@ -1294,10 +1296,14 @@ void remap_struct(unsigned int gtypes_nr
 	/* Remap [unsigned] long fields */
 	d = gtypes_nr[0] - ftypes_nr[0];
 	if (d) {
+		n = MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH,
+			    g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH);
+		if ((ftypes_nr[0] * ULL_ALIGNMENT_WIDTH >= b_size) ||
+		    (gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + n > b_size) ||
+		    (ftypes_nr[0] * ULL_ALIGNMENT_WIDTH + n > b_size))
+			return;
 		memmove(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH,
-			((char *) ps) + ftypes_nr[0] * ULL_ALIGNMENT_WIDTH,
-			MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH,
-				g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH));
+			((char *) ps) + ftypes_nr[0] * ULL_ALIGNMENT_WIDTH, n);
 		if (d > 0) {
 			memset(((char *) ps) + ftypes_nr[0] * ULL_ALIGNMENT_WIDTH,
 			       0, d * ULL_ALIGNMENT_WIDTH);
@@ -1306,14 +1312,21 @@ void remap_struct(unsigned int gtypes_nr
 	/* Remap [unsigned] int fields */
 	d = gtypes_nr[1] - ftypes_nr[1];
 	if (d) {
+		n = MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH
+				   - ftypes_nr[1] * UL_ALIGNMENT_WIDTH,
+			    g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
+				   - gtypes_nr[1] * UL_ALIGNMENT_WIDTH);
+		if ((gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     ftypes_nr[1] * UL_ALIGNMENT_WIDTH >= b_size) ||
+		    (gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     gtypes_nr[1] * UL_ALIGNMENT_WIDTH + n > b_size) ||
+		    (gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     ftypes_nr[1] * UL_ALIGNMENT_WIDTH + n > b_size))
+			return;
 		memmove(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
 				      + gtypes_nr[1] * UL_ALIGNMENT_WIDTH,
 			((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
-				      + ftypes_nr[1] * UL_ALIGNMENT_WIDTH,
-			MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH
-				       - ftypes_nr[1] * UL_ALIGNMENT_WIDTH,
-				g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
-				       - gtypes_nr[1] * UL_ALIGNMENT_WIDTH));
+				      + ftypes_nr[1] * UL_ALIGNMENT_WIDTH, n);
 		if (d > 0) {
 			memset(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
 					     + ftypes_nr[1] * UL_ALIGNMENT_WIDTH,
@@ -1323,18 +1336,28 @@ void remap_struct(unsigned int gtypes_nr
 	/* Remap possible fields (like strings of chars) following int fields */
 	d = gtypes_nr[2] - ftypes_nr[2];
 	if (d) {
+		n = MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH
+				   - ftypes_nr[1] * UL_ALIGNMENT_WIDTH
+				   - ftypes_nr[2] * U_ALIGNMENT_WIDTH,
+			    g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
+				   - gtypes_nr[1] * UL_ALIGNMENT_WIDTH
+				   - gtypes_nr[2] * U_ALIGNMENT_WIDTH);
+		if ((gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     gtypes_nr[1] * UL_ALIGNMENT_WIDTH +
+		     ftypes_nr[2] * U_ALIGNMENT_WIDTH >= b_size) ||
+		    (gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     gtypes_nr[1] * UL_ALIGNMENT_WIDTH +
+		     gtypes_nr[2] * U_ALIGNMENT_WIDTH + n > b_size) ||
+		    (gtypes_nr[0] * ULL_ALIGNMENT_WIDTH +
+		     gtypes_nr[1] * UL_ALIGNMENT_WIDTH +
+		     ftypes_nr[2] * U_ALIGNMENT_WIDTH + n > b_size))
+			return;
 		memmove(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
 				      + gtypes_nr[1] * UL_ALIGNMENT_WIDTH
 				      + gtypes_nr[2] * U_ALIGNMENT_WIDTH,
 			((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
 				      + gtypes_nr[1] * UL_ALIGNMENT_WIDTH
-				      + ftypes_nr[2] * U_ALIGNMENT_WIDTH,
-			MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH
-				       - ftypes_nr[1] * UL_ALIGNMENT_WIDTH
-				       - ftypes_nr[2] * U_ALIGNMENT_WIDTH,
-				g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
-				       - gtypes_nr[1] * UL_ALIGNMENT_WIDTH
-				       - gtypes_nr[2] * U_ALIGNMENT_WIDTH));
+				      + ftypes_nr[2] * U_ALIGNMENT_WIDTH, n);
 		if (d > 0) {
 			memset(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH
 					     + gtypes_nr[1] * UL_ALIGNMENT_WIDTH
@@ -1396,6 +1419,7 @@ int sa_fread(int ifd, void *buffer, size
  * @endian_mismatch
  *		TRUE if data read from file don't match current machine's
  *		endianness.
+ * @b_size     @buffer size.
  *
  * OUT:
  * @record_hdr	Record header for current sample.
@@ -1405,7 +1429,8 @@ int sa_fread(int ifd, void *buffer, size
  ***************************************************************************
  */
 int read_record_hdr(int ifd, void *buffer, struct record_header *record_hdr,
-		    struct file_header *file_hdr, int arch_64, int endian_mismatch)
+		    struct file_header *file_hdr, int arch_64, int endian_mismatch,
+		    size_t b_size)
 {
 	if (sa_fread(ifd, buffer, (size_t) file_hdr->rec_size, SOFT_SIZE))
 		/* End of sa data file */
@@ -1413,7 +1438,7 @@ int read_record_hdr(int ifd, void *buffe
 
 	/* Remap record header structure to that expected by current version */
 	remap_struct(rec_types_nr, file_hdr->rec_types_nr, buffer,
-		     file_hdr->rec_size, RECORD_HEADER_SIZE);
+		     file_hdr->rec_size, RECORD_HEADER_SIZE, b_size);
 	memcpy(record_hdr, buffer, RECORD_HEADER_SIZE);
 
 	/* Normalize endianness */
@@ -1622,7 +1647,7 @@ void read_file_stat_bunch(struct activit
 		for (j = 0; j < (nr_value * act[p]->nr2); j++) {
 			remap_struct(act[p]->gtypes_nr, act[p]->ftypes_nr,
 				     (char *) act[p]->buf[curr] + j * act[p]->msize,
-				     act[p]->fsize, act[p]->msize);
+				     act[p]->fsize, act[p]->msize, act[p]->msize);
 		}
 	}
 }
@@ -1804,7 +1829,7 @@ void check_file_actlst(int *ifd, char *d
 	 * then copy its contents to the expected  structure.
 	 */
 	remap_struct(hdr_types_nr, file_magic->hdr_types_nr, buffer,
-		     file_magic->header_size, FILE_HEADER_SIZE);
+		     file_magic->header_size, FILE_HEADER_SIZE, file_magic->header_size);
 	memcpy(file_hdr, buffer, FILE_HEADER_SIZE);
 	free(buffer);
 	buffer = NULL;
@@ -1853,7 +1878,7 @@ void check_file_actlst(int *ifd, char *d
 		* then copy its contents to the expected structure.
 		*/
 		remap_struct(act_types_nr, file_hdr->act_types_nr, buffer,
-			     file_hdr->act_size, FILE_ACTIVITY_SIZE);
+			     file_hdr->act_size, FILE_ACTIVITY_SIZE, file_hdr->act_size);
 		memcpy(fal, buffer, FILE_ACTIVITY_SIZE);
 
 		/* Normalize endianness for file_activity structures */
Index: sysstat-12.0.2/sadf.c
===================================================================
--- sysstat-12.0.2.orig/sadf.c
+++ sysstat-12.0.2/sadf.c
@@ -232,7 +232,7 @@ int read_next_sample(int ifd, int action
 
 	/* Read current record */
 	if (read_record_hdr(ifd, rec_hdr_tmp, &record_hdr[curr], &file_hdr,
-			    arch_64, endian_mismatch))
+			    arch_64, endian_mismatch, sizeof(rec_hdr_tmp)))
 		/* End of sa file */
 		return TRUE;
 
Index: sysstat-12.0.2/sar.c
===================================================================
--- sysstat-12.0.2.orig/sar.c
+++ sysstat-12.0.2/sar.c
@@ -727,6 +727,7 @@ void read_sadc_stat_bunch(int curr)
  * @endian_mismatch
  *		TRUE if file's data don't match current machine's endianness.
  * @arch_64	TRUE if file's data come from a 64 bit machine.
+ * @b_size	Size of @rec_hdr_tmp buffer.
  *
  * OUT:
  * @curr	Index in array for next sample statistics.
@@ -740,7 +741,7 @@ void handle_curr_act_stats(int ifd, off_
 			   int rows, unsigned int act_id, int *reset,
 			   struct file_activity *file_actlst, char *file,
 			   struct file_magic *file_magic, void *rec_hdr_tmp,
-			   int endian_mismatch, int arch_64)
+			   int endian_mismatch, int arch_64, size_t b_size)
 {
 	int p, reset_cd;
 	unsigned long lines = 0;
@@ -774,7 +775,7 @@ void handle_curr_act_stats(int ifd, off_
 		 * Start with reading current sample's record header.
 		 */
 		*eosaf = read_record_hdr(ifd, rec_hdr_tmp, &record_hdr[*curr],
-					 &file_hdr, arch_64, endian_mismatch);
+					 &file_hdr, arch_64, endian_mismatch, b_size);
 		rtype = record_hdr[*curr].record_type;
 
 		if (!*eosaf && (rtype != R_RESTART) && (rtype != R_COMMENT)) {
@@ -1003,7 +1004,7 @@ void read_stats_from_file(char from_file
 		 */
 		do {
 			if (read_record_hdr(ifd, rec_hdr_tmp, &record_hdr[0], &file_hdr,
-					    arch_64, endian_mismatch)) {
+					    arch_64, endian_mismatch, sizeof(rec_hdr_tmp))) {
 				/* End of sa data file */
 				return;
 			}
@@ -1069,7 +1070,7 @@ void read_stats_from_file(char from_file
 				handle_curr_act_stats(ifd, fpos, &curr, &cnt, &eosaf, rows,
 						      act[p]->id, &reset, file_actlst,
 						      from_file, &file_magic, rec_hdr_tmp,
-						      endian_mismatch, arch_64);
+						      endian_mismatch, arch_64, sizeof(rec_hdr_tmp));
 			}
 			else {
 				unsigned int optf, msk;
@@ -1083,7 +1084,7 @@ void read_stats_from_file(char from_file
 						handle_curr_act_stats(ifd, fpos, &curr, &cnt, &eosaf,
 								      rows, act[p]->id, &reset, file_actlst,
 								      from_file, &file_magic, rec_hdr_tmp,
-								      endian_mismatch, arch_64);
+								      endian_mismatch, arch_64, sizeof(rec_hdr_tmp));
 						act[p]->opt_flags = optf;
 					}
 				}
@@ -1095,7 +1096,7 @@ void read_stats_from_file(char from_file
 			do {
 				/* Read next record header */
 				eosaf = read_record_hdr(ifd, rec_hdr_tmp, &record_hdr[curr],
-							&file_hdr, arch_64, endian_mismatch);
+							&file_hdr, arch_64, endian_mismatch, sizeof(rec_hdr_tmp));
 				rtype = record_hdr[curr].record_type;
 
 				if (!eosaf && (rtype != R_RESTART) && (rtype != R_COMMENT)) {

Places

File sysstat-12.0.2-CVE-2018-19416-and-CVE-2018-19517.patch of Package sysstat (Revision 2c5ba3b60b9ceeff3ef79f99f6b52bcd)

Places