MidiManagerUsb should not trust indices provided by renderer.
MidiManagerUsb::DispatchSendMidiData takes |port_index| parameter. As it is
provided by a renderer possibly under the control of an attacker, we must
validate the given index before using it.
BUG=456516
Review URL: https://codereview.chromium.org/907793002
Cr-Commit-Position: refs/heads/master@{#315303}
diff --git a/media/midi/midi_manager_usb.cc b/media/midi/midi_manager_usb.cc
index fcefdf5a..63c241b9 100644
--- a/media/midi/midi_manager_usb.cc
+++ b/media/midi/midi_manager_usb.cc
@@ -43,7 +43,13 @@
uint32_t port_index,
const std::vector<uint8>& data,
double timestamp) {
- DCHECK_LT(port_index, output_streams_.size());
+ if (port_index >= output_streams_.size()) {
+ // |port_index| is provided by a renderer so we can't believe that it is
+ // in the valid range.
+ // TODO(toyoshim): Move this check to MidiHost and kill the renderer when
+ // it fails.
+ return;
+ }
output_streams_[port_index]->Send(data);
client->AccumulateMidiBytesSent(data.size());
}
diff --git a/media/midi/midi_manager_usb_unittest.cc b/media/midi/midi_manager_usb_unittest.cc
index 4df64a6..8552403 100644
--- a/media/midi/midi_manager_usb_unittest.cc
+++ b/media/midi/midi_manager_usb_unittest.cc
@@ -316,6 +316,50 @@
logger_.TakeLog());
}
+TEST_F(MidiManagerUsbTest, SendFromCompromizedRenderer) {
+ scoped_ptr<FakeUsbMidiDevice> device(new FakeUsbMidiDevice(&logger_));
+ FakeMidiManagerClient client(&logger_);
+ uint8 descriptor[] = {
+ 0x12, 0x01, 0x10, 0x01, 0x00, 0x00, 0x00, 0x08, 0x86, 0x1a,
+ 0x2d, 0x75, 0x54, 0x02, 0x00, 0x02, 0x00, 0x01, 0x09, 0x02,
+ 0x75, 0x00, 0x02, 0x01, 0x00, 0x80, 0x30, 0x09, 0x04, 0x00,
+ 0x00, 0x00, 0x01, 0x01, 0x00, 0x00, 0x09, 0x24, 0x01, 0x00,
+ 0x01, 0x09, 0x00, 0x01, 0x01, 0x09, 0x04, 0x01, 0x00, 0x02,
+ 0x01, 0x03, 0x00, 0x00, 0x07, 0x24, 0x01, 0x00, 0x01, 0x51,
+ 0x00, 0x06, 0x24, 0x02, 0x01, 0x02, 0x00, 0x06, 0x24, 0x02,
+ 0x01, 0x03, 0x00, 0x06, 0x24, 0x02, 0x02, 0x06, 0x00, 0x09,
+ 0x24, 0x03, 0x01, 0x07, 0x01, 0x06, 0x01, 0x00, 0x09, 0x24,
+ 0x03, 0x02, 0x04, 0x01, 0x02, 0x01, 0x00, 0x09, 0x24, 0x03,
+ 0x02, 0x05, 0x01, 0x03, 0x01, 0x00, 0x09, 0x05, 0x02, 0x02,
+ 0x20, 0x00, 0x00, 0x00, 0x00, 0x06, 0x25, 0x01, 0x02, 0x02,
+ 0x03, 0x09, 0x05, 0x82, 0x02, 0x20, 0x00, 0x00, 0x00, 0x00,
+ 0x05, 0x25, 0x01, 0x01, 0x07,
+ };
+
+ device->SetDescriptor(ToVector(descriptor));
+ uint8 data[] = {
+ 0x90, 0x45, 0x7f,
+ 0xf0, 0x00, 0x01, 0xf7,
+ };
+
+ Initialize();
+ ScopedVector<UsbMidiDevice> devices;
+ devices.push_back(device.release());
+ EXPECT_FALSE(IsInitializationCallbackInvoked());
+ RunCallbackUntilCallbackInvoked(true, &devices);
+ EXPECT_EQ(MIDI_OK, GetInitializationResult());
+ ASSERT_EQ(2u, manager_->output_streams().size());
+ EXPECT_EQ("UsbMidiDevice::GetDescriptor\n", logger_.TakeLog());
+
+ // The specified port index is invalid. The manager must ignore the request.
+ manager_->DispatchSendMidiData(&client, 99, ToVector(data), 0);
+ EXPECT_EQ("", logger_.TakeLog());
+
+ // The specified port index is invalid. The manager must ignore the request.
+ manager_->DispatchSendMidiData(&client, 2, ToVector(data), 0);
+ EXPECT_EQ("", logger_.TakeLog());
+}
+
TEST_F(MidiManagerUsbTest, Receive) {
scoped_ptr<FakeUsbMidiDevice> device(new FakeUsbMidiDevice(&logger_));
uint8 descriptor[] = {
