mirror/JUCE
1
0
Fork 0
mirror of https://github.com/juce-framework/JUCE.git synced 2026-01-10 23:44:24 +00:00
Code Activity

MidiFile: Make file-reading more robust

Browse source 
This commit adds tests and fixes some potential crashes caused by
out-of-bounds reads.
This commit is contained in:
reuk 2020-10-20 12:25:11 +01:00
parent a98dc7553b
commit 0943291990
4 changed files with 669 additions and 103 deletions
Download patch file Download diff file Expand all files Collapse all files

215
modules/juce_audio_basics/midi/juce_MidiMessage.cpp
View file

@ -57,6 +57,32 @@ uint16 MidiMessage::pitchbendToPitchwheelPos (const float pitchbend,
}
//==============================================================================
MidiMessage::VariableLengthValue MidiMessage::readVariableLengthValue (const uint8* data, int maxBytesToUse) noexcept
{
uint32 v = 0;
// The largest allowable variable-length value is 0x0f'ff'ff'ff which is
// represented by the 4-byte stream 0xff 0xff 0xff 0x7f.
// Longer bytestreams risk overflowing a 32-bit signed int.
const auto limit = jmin (maxBytesToUse, 4);
for (int numBytesUsed = 0; numBytesUsed < limit; ++numBytesUsed)
{
const auto i = data[numBytesUsed];
v = (v << 7) + (i & 0x7f);
if (! (i & 0x80))
return { (int) v, numBytesUsed + 1 };
}
// If this is hit, the input was malformed. Either there were not enough
// bytes of input to construct a full value, or no terminating byte was
// found. This implementation only supports variable-length values of up
// to four bytes.
jassertfalse;
return {};
}
int MidiMessage::readVariableLengthVal (const uint8* data, int& numBytesUsed) noexcept
{
numBytesUsed = 0;
@ -224,16 +250,8 @@ MidiMessage::MidiMessage (const void* srcData, int sz, int& numBytesUsed, const
}
else if (byte == 0xff)
{
if (sz == 1)
{
size = 1;
}
else
{
int n;
const int bytesLeft = readVariableLengthVal (src + 1, n);
size = jmin (sz + 1, n + 2 + bytesLeft);
}
const auto bytesLeft = readVariableLengthValue (src + 1, sz - 1);
size = jmin (sz + 1, bytesLeft.bytesUsed + 2 + bytesLeft.value);
auto dest = allocateSpace (size);
*dest = (uint8) byte;
@ -682,7 +700,7 @@ bool MidiMessage::isActiveSense() const noexcept { return *getRawData() == 0x
int MidiMessage::getMetaEventType() const noexcept
{
auto data = getRawData();
return *data != 0xff ? -1 : data[1];
return (size < 2 || *data != 0xff) ? -1 : data[1];
}
int MidiMessage::getMetaEventLength() const noexcept
@ -691,8 +709,8 @@ int MidiMessage::getMetaEventLength() const noexcept
if (*data == 0xff)
{
int n;
return jmin (size - 2, readVariableLengthVal (data + 2, n));
const auto var = readVariableLengthValue (data + 2, size - 2);
return jmax (0, jmin (size - 2 - var.bytesUsed, var.value));
}
return 0;
@ -702,10 +720,9 @@ const uint8* MidiMessage::getMetaEventData() const noexcept
{
jassert (isMetaEvent());
int n;
auto d = getRawData() + 2;
readVariableLengthVal (d, n);
return d + n;
const auto var = readVariableLengthValue (d, size - 2);
return d + var.bytesUsed;
}
bool MidiMessage::isTrackMetaEvent() const noexcept { return getMetaEventType() == 0; }
@ -1141,4 +1158,170 @@ const char* MidiMessage::getControllerName (const int n)
return isPositiveAndBelow (n, numElementsInArray (names)) ? names[n] : nullptr;
}
//==============================================================================
//==============================================================================
#if JUCE_UNIT_TESTS
struct MidiMessageTest : public UnitTest
{
MidiMessageTest()
: UnitTest ("MidiMessage", UnitTestCategories::midi)
{}
void runTest() override
{
using std::begin;
using std::end;
beginTest ("ReadVariableLengthValue should return valid, backward-compatible results");
{
const std::vector<uint8> inputs[]
{
{ 0x00 },
{ 0x40 },
{ 0x7f },
{ 0x81, 0x00 },
{ 0xc0, 0x00 },
{ 0xff, 0x7f },
{ 0x81, 0x80, 0x00 },
{ 0xc0, 0x80, 0x00 },
{ 0xff, 0xff, 0x7f },
{ 0x81, 0x80, 0x80, 0x00 },
{ 0xc0, 0x80, 0x80, 0x00 },
{ 0xff, 0xff, 0xff, 0x7f }
};
const int outputs[]
{
0x00,
0x40,
0x7f,
0x80,
0x2000,
0x3fff,
0x4000,
0x100000,
0x1fffff,
0x200000,
0x8000000,
0xfffffff,
};
expectEquals (std::distance (begin (inputs), end (inputs)),
std::distance (begin (outputs), end (outputs)));
size_t index = 0;
JUCE_BEGIN_IGNORE_WARNINGS_GCC_LIKE ("-Wdeprecated-declarations")
JUCE_BEGIN_IGNORE_WARNINGS_MSVC (4996)
for (const auto& input : inputs)
{
auto copy = input;
while (copy.size() < 16)
copy.push_back (0);
const auto result = MidiMessage::readVariableLengthValue (copy.data(),
(int) copy.size());
expectEquals (result.value, outputs[index]);
expectEquals (result.bytesUsed, (int) inputs[index].size());
int legacyNumUsed = 0;
const auto legacyResult = MidiMessage::readVariableLengthVal (copy.data(),
legacyNumUsed);
expectEquals (result.value, legacyResult);
expectEquals (result.bytesUsed, legacyNumUsed);
++index;
}
JUCE_END_IGNORE_WARNINGS_GCC_LIKE
JUCE_END_IGNORE_WARNINGS_MSVC
}
beginTest ("ReadVariableLengthVal should return 0 if input is truncated");
{
for (size_t i = 0; i != 16; ++i)
{
std::vector<uint8> input;
input.resize (i, 0xFF);
const auto result = MidiMessage::readVariableLengthValue (input.data(),
(int) input.size());
expectEquals (result.value, 0);
expectEquals (result.bytesUsed, 0);
}
}
const std::vector<uint8> metaEvents[]
{
// Format is 0xff, followed by a 'kind' byte, followed by a variable-length
// 'data-length' value, followed by that many data bytes
{ 0xff, 0x00, 0x02, 0x00, 0x00 }, // Sequence number
{ 0xff, 0x01, 0x00 }, // Text event
{ 0xff, 0x02, 0x00 }, // Copyright notice
{ 0xff, 0x03, 0x00 }, // Track name
{ 0xff, 0x04, 0x00 }, // Instrument name
{ 0xff, 0x05, 0x00 }, // Lyric
{ 0xff, 0x06, 0x00 }, // Marker
{ 0xff, 0x07, 0x00 }, // Cue point
{ 0xff, 0x20, 0x01, 0x00 }, // Channel prefix
{ 0xff, 0x2f, 0x00 }, // End of track
{ 0xff, 0x51, 0x03, 0x01, 0x02, 0x03 }, // Set tempo
{ 0xff, 0x54, 0x05, 0x01, 0x02, 0x03, 0x04, 0x05 }, // SMPTE offset
{ 0xff, 0x58, 0x04, 0x01, 0x02, 0x03, 0x04 }, // Time signature
{ 0xff, 0x59, 0x02, 0x01, 0x02 }, // Key signature
{ 0xff, 0x7f, 0x00 }, // Sequencer-specific
};
beginTest ("MidiMessage data constructor works for well-formed meta-events");
{
const auto status = (uint8) 0x90;
for (const auto& input : metaEvents)
{
int bytesUsed = 0;
const MidiMessage msg (input.data(), (int) input.size(), bytesUsed, status);
expect (msg.isMetaEvent());
expectEquals (msg.getMetaEventLength(), (int) input.size() - 3);
expectEquals (msg.getMetaEventType(), (int) input[1]);
}
}
beginTest ("MidiMessage data constructor works for malformed meta-events");
{
const auto status = (uint8) 0x90;
const auto runTest = [&] (const std::vector<uint8>& input)
{
int bytesUsed = 0;
const MidiMessage msg (input.data(), (int) input.size(), bytesUsed, status);
expect (msg.isMetaEvent());
expectEquals (msg.getMetaEventLength(), jmax (0, (int) input.size() - 3));
expectEquals (msg.getMetaEventType(), input.size() >= 2 ? input[1] : -1);
};
runTest ({ 0xff });
for (const auto& input : metaEvents)
{
auto copy = input;
copy[2] = 0x40; // Set the size of the message to more bytes than are present
runTest (copy);
}
}
}
};
static MidiMessageTest midiMessageTests;
#endif
} // namespace juce