sa2.cpp: Add missing checks in load()

The number of patterns, order length, and restart position are read from the file and used without checking. Add a check that rejects the file if these values anr not in the expected range to avoid OOB accesses or other unwanted behavior later. Also the track data is read until the end of the file, possibly overflowing the tracks array. Add checks to stop reading when the allocated number of patterns is filled. These changes fix crashes in rewind() for the follwing fuzzing samples: i-110_049.sa2, i-110_050.sa2, i-110_067.sat, i-110_081.sa2, i-110_082.sa2, i-110_106.sat, i-110_134.sat, i-110_140.sat, i-110_142.sat, i-110_148.sa2, i-110_150.sa2, i-110_162.sa2, i-110_163.sa2, i-110_167.sa2, i-110_197.sa2, and a crash in load() for sample i-110_149.sa2. Bug: adplug#110
dan-giddins · Jun 10, 2020 · ab98663 · ab98663
1 parent aedd31c
commit ab98663
Showing 1 changed file with 9 additions and 3 deletions.
diff --git a/src/sa2.cpp b/src/sa2.cpp
@@ -132,6 +132,12 @@ bool Csa2Loader::load(const std::string &filename, const CFileProvider &fp)
 
   // infos
   nop = f->readInt(2); length = f->readInt(1); restartpos = f->readInt(1);
+  if (nop < 1 || nop > 64 ||
+      length < 1 || length > 128 ||
+      restartpos >= length) {
+    fp.close(f);
+    return false;
+  }
 
   // bpm
   bpm = f->readInt(2);
@@ -166,7 +172,7 @@ bool Csa2Loader::load(const std::string &filename, const CFileProvider &fp)
   // track data
   if(sat_type & HAS_OLDPATTERNS) {
     i = 0;
-    while(!f->ateof()) {
+    while (i < 64 * 9 && !f->ateof()) {
       for(j=0;j<64;j++) {
 	for(k=0;k<9;k++) {
 	  buf = f->readInt(1);
@@ -182,7 +188,7 @@ bool Csa2Loader::load(const std::string &filename, const CFileProvider &fp)
   } else
     if(sat_type & HAS_V7PATTERNS) {
       i = 0;
-      while(!f->ateof()) {
+      while (i < 64 * 9 && !f->ateof()) {
 	for(j=0;j<64;j++) {
 	  for(k=0;k<9;k++) {
 	    buf = f->readInt(1);
@@ -200,7 +206,7 @@ bool Csa2Loader::load(const std::string &filename, const CFileProvider &fp)
       }
     } else {
       i = 0;
-      while(!f->ateof()) {
+      while (i < 64 * 9 && !f->ateof()) {
 	for(j=0;j<64;j++) {
 	  buf = f->readInt(1);
 	  tracks[i][j].note = buf >> 1;