View Issue Details

IDProjectCategoryView StatusLast Update
0001191OpenMPT[All Projects] File Format Supportpublic2019-01-17 21:18
ReportermanxAssigned Tomanx 
PriorityhighSeveritycrashReproducibilityalways
Status resolvedResolutionfixed 
Product VersionOpenMPT 1.28.02.00 / libopenmpt 0.4.1 (upgrade first) 
Target VersionOpenMPT 1.28.03.00 / libopenmpt 0.4.3 (current stable)Fixed in VersionOpenMPT 1.28.03.00 / libopenmpt 0.4.3 (current stable) 
Summary0001191: out of bounds vector access in DSM loader
Description

https://modarchive.org/index.php?request=view_by_moduleid&query=178886 crashes in Load_dsm.cpp:267 when row==63 at the start of the loop iteration.

Additional Information

https://redmine.audacious-media-player.org/issues/848

TagsNo tags attached.
Has the bug occurred in previous versions?also affects libopenmpt 0.3 and 0.2
Tested code revision (in case you know it)

Activities

manx

manx

2019-01-17 20:37

administrator   ~0003819

This is not an actual out-of-bound read because the pointer in question is never dereferenced. However, construction of the pointer involves undefined behaviour (and triggers assertions in debug STLs) due to accessing the vector with an invalid index.

manx

manx

2019-01-17 20:37

administrator  

fix-dsm-out-of-bounds-vector-access-bug1191-v2.patch (574 bytes)
Index: soundlib/Load_dsm.cpp
===================================================================
--- soundlib/Load_dsm.cpp	(revision 11205)
+++ soundlib/Load_dsm.cpp	(working copy)
@@ -257,14 +257,13 @@
 
 			ModCommand dummy = ModCommand::Empty();
 			ROWINDEX row = 0;
-			PatternRow rowBase = Patterns[patNum].GetRow(0);
 			while(chunk.CanRead(1) && row < 64)
 			{
+				PatternRow rowBase = Patterns[patNum].GetRow(row);
 				uint8 flag = chunk.ReadUint8();
 				if(!flag)
 				{
 					row++;
-					rowBase = Patterns[patNum].GetRow(row);
 					continue;
 				}
 
manx

manx

2019-01-17 21:02

administrator  

fix-dsm-out-of-bounds-vector-access-bug1191-v3.patch (778 bytes)
Index: soundlib/Load_dsm.cpp
===================================================================
--- soundlib/Load_dsm.cpp	(revision 11205)
+++ soundlib/Load_dsm.cpp	(working copy)
@@ -257,7 +257,6 @@
 
 			ModCommand dummy = ModCommand::Empty();
 			ROWINDEX row = 0;
-			PatternRow rowBase = Patterns[patNum].GetRow(0);
 			while(chunk.CanRead(1) && row < 64)
 			{
 				uint8 flag = chunk.ReadUint8();
@@ -264,12 +263,11 @@
 				if(!flag)
 				{
 					row++;
-					rowBase = Patterns[patNum].GetRow(row);
 					continue;
 				}
 
 				CHANNELINDEX chn = (flag & 0x0F);
-				ModCommand &m = (chn < GetNumChannels() ? rowBase[chn] : dummy);
+				ModCommand &m = (chn < GetNumChannels() ? *Patterns[patNum].GetpModCommand(row, chn) : dummy);
 
 				if(flag & 0x80)
 				{
manx

manx

2019-01-17 21:18

administrator   ~0003820

Fixed in r11208 (OpenMPT 1.29 / libopenmpt 0.5), r11209 (OpenMPT 1.28, libopenmpt 0.4), r11210 (libopenmpt 0.3), r11211 (libopenmpt 0.2).

Issue History

Date Modified Username Field Change
2019-01-17 20:04 manx New Issue
2019-01-17 20:04 manx Status new => assigned
2019-01-17 20:04 manx Assigned To => Saga Musix
2019-01-17 20:34 manx Summary out of bounds read in DSM loader => out of bounds vector access in DSM loader
2019-01-17 20:37 manx Note Added: 0003819
2019-01-17 20:37 manx File Added: fix-dsm-out-of-bounds-vector-access-bug1191-v2.patch
2019-01-17 20:38 manx Assigned To Saga Musix => manx
2019-01-17 21:02 manx File Added: fix-dsm-out-of-bounds-vector-access-bug1191-v3.patch
2019-01-17 21:18 manx Status assigned => resolved
2019-01-17 21:18 manx Resolution open => fixed
2019-01-17 21:18 manx Fixed in Version => OpenMPT 1.28.03.00 / libopenmpt 0.4.3 (current stable)
2019-01-17 21:18 manx Note Added: 0003820