Add bound checks in AddExporterStat #174

phaag · Aug 5, 2019 · 859ea2c · 859ea2c
1 parent 7dbd3eb
commit 859ea2c
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 7 deletions.
diff --git a/ChangeLog b/ChangeLog
@@ -2,6 +2,7 @@
 - Fix nfdump.1 man page. #175
 - Fix off by 1 array. #173
 - Fix use after free in ModifyCompressFile
+- Add bound checks in AddExporterStat #174
 
 2019-07-31
 - Add early record size sanity check also for nfprofile, nfanon and nfreplay

diff --git a/bin/exporter.c b/bin/exporter.c
@@ -225,7 +225,18 @@ generic_sampler_t	**sampler;
 int AddExporterStat(exporter_stats_record_t *stat_record) {
 int i, use_copy;
 exporter_stats_record_t *rec;
-size_t size;
+size_t required;
+
+	if ( stat_record->header.size < sizeof(exporter_stats_record_t) ) {
+		LogError("Corrupt exporter record in %s line %d\n", __FILE__, __LINE__);
+		return 0;
+	}
+
+	required = sizeof(exporter_stats_record_t) + (stat_record->stat_count-1) * sizeof(struct exporter_stat_s);
+	if ((stat_record->stat_count == 0) || (stat_record->header.size != required)) {
+		LogError("Corrupt exporter record in %s line %d\n", __FILE__, __LINE__);
+		return 0;
+	}
 
 	// 64bit counters can be potentially unaligned
 	if ( ((ptrdiff_t)stat_record & 0x7) != 0 ) {
@@ -241,13 +252,12 @@ size_t size;
 		use_copy = 0;
 	}
 
-	size = sizeof(exporter_stats_record_t) + (rec->stat_count -1) * sizeof(struct exporter_stat_s);
-	if ( size > rec->header.size ) {
-		LogError("Corrupt exporter record in %s line %d\n", __FILE__, __LINE__);
-		return 0;
-	}
 	for (i=0; i<rec->stat_count; i++ ) {
 		uint32_t id = rec->stat[i].sysid;
+		if ( id >= MAX_EXPORTERS ) {
+			LogError("Corrupt exporter record in %s line %d\n", __FILE__, __LINE__);
+			return 0;
+		}
 		if ( !exporter_list[id] ) {
 			LogError("Exporter SysID: %u not found! - Skip stat record record.\n");
 			continue;

diff --git a/bin/nfdump.c b/bin/nfdump.c
@@ -532,7 +532,7 @@ int	v1_map_done = 0;
 		record_ptr = nffile_r->buff_ptr;
 		for ( i=0; i < nffile_r->block_header->NumRecords; i++ ) {
 			flow_record = record_ptr;
-			if ( (sumSize + record_ptr->size) > ret || (record_ptr->size < COMMON_RECORD_DATA_SIZE) ) {
+			if ( (sumSize + record_ptr->size) > ret || (record_ptr->size < sizeof(record_header_t)) ) {
 				LogError("Corrupt data file. Inconsistent block size in %s line %d\n", __FILE__, __LINE__);
 				exit(255);
 			}