Commit 5576cbc1 authored by yhirano's avatar yhirano Committed by Commit bot

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}
parent 3a105408
...@@ -43,7 +43,13 @@ void MidiManagerUsb::DispatchSendMidiData(MidiManagerClient* client, ...@@ -43,7 +43,13 @@ void MidiManagerUsb::DispatchSendMidiData(MidiManagerClient* client,
uint32_t port_index, uint32_t port_index,
const std::vector<uint8>& data, const std::vector<uint8>& data,
double timestamp) { 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); output_streams_[port_index]->Send(data);
client->AccumulateMidiBytesSent(data.size()); client->AccumulateMidiBytesSent(data.size());
} }
......
...@@ -316,6 +316,50 @@ TEST_F(MidiManagerUsbTest, Send) { ...@@ -316,6 +316,50 @@ TEST_F(MidiManagerUsbTest, Send) {
logger_.TakeLog()); 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) { TEST_F(MidiManagerUsbTest, Receive) {
scoped_ptr<FakeUsbMidiDevice> device(new FakeUsbMidiDevice(&logger_)); scoped_ptr<FakeUsbMidiDevice> device(new FakeUsbMidiDevice(&logger_));
uint8 descriptor[] = { uint8 descriptor[] = {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment