View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0001191 | OpenMPT | File Format Support | public | 2019-01-17 20:04 | 2019-01-17 21:18 |
| Reporter | manx | Assigned To | manx | ||
| Priority | high | Severity | crash | Reproducibility | always |
| Status | resolved | Resolution | fixed | ||
| Product Version | OpenMPT 1.28.02.00 / libopenmpt 0.4.1 (upgrade first) | ||||
| Target Version | OpenMPT 1.28.03.00 / libopenmpt 0.4.3 (upgrade first) | Fixed in Version | OpenMPT 1.28.03.00 / libopenmpt 0.4.3 (upgrade first) | ||
| Summary | 0001191: 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 | |||||
| Tags | No tags attached. | ||||
| Attached Files | 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;
}
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)
{
| ||||
| Has the bug occurred in previous versions? | also affects libopenmpt 0.3 and 0.2 | ||||
| Tested code revision (in case you know it) | |||||
 |
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. |
|
|
Fixed in r11208 (OpenMPT 1.29 / libopenmpt 0.5), r11209 (OpenMPT 1.28, libopenmpt 0.4), r11210 (libopenmpt 0.3), r11211 (libopenmpt 0.2). |
| 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 (upgrade first) |
| 2019-01-17 21:18 | manx | Note Added: 0003820 |
