Add testcase for KeyAddrEventQueue bounds checking failure

`KeyAddrEventQueue::remove()` fails to confirm that the current queue is empty
before decrementing the length and shifting entries in the queue arrays.  If
`remove()` or `shift()` is called when the queue is empty, `length_` gets
decremented from 0 to 255 (because it's unsigned), and then a large section of
memory gets shifted, mostly out of bounds of the event queue arrays, and
probably wreaking havoc with any number of things.

The plugin in this testcase should trigger this bug, and is detectable because
it affects the value for the current time.  It's not guaranteed to detect this
bug, but it seems to be fairly consistent.

Signed-off-by: Michael Richters <gedankenexperimenter@gmail.com>
pull/1107/head
Michael Richters 3 years ago
parent d59604e98b
commit 4480b08b2d
No known key found for this signature in database
GPG Key ID: 1288FD13E4EEF0C0

@ -0,0 +1,71 @@
/* -*- mode: c++ -*-
* Copyright (C) 2022 Keyboard.io, Inc.
*
* This program is free software: you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
* Foundation, version 3.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
* details.
*
* You should have received a copy of the GNU General Public License along with
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "Kaleidoscope.h"
// *INDENT-OFF*
KEYMAPS(
[0] = KEYMAP_STACKED
(
XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX
,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX ,XXX ,XXX ,XXX
,XXX ,XXX ,XXX ,XXX
,XXX
)
) // KEYMAPS(
// *INDENT-ON*
#include <kaleidoscope/KeyAddrEventQueue.h>
namespace kaleidoscope {
namespace plugin {
class QueueLeaker : public Plugin {
public:
EventHandlerResult afterEachCycle() {
// Shift queue without checking to be sure that queue is not empty. If
// KeyAddrEventQueue uses an unsigned integer for queue length, this will
// result in some bad behavior.
queue_.shift();
return EventHandlerResult::OK;
}
private:
KeyAddrEventQueue<8> queue_;
};
} // namespace plugin
} // namespace kaleidoscope
kaleidoscope::plugin::QueueLeaker QueueLeaker;
KALEIDOSCOPE_INIT_PLUGINS(QueueLeaker);
void setup() {
Kaleidoscope.setup();
}
void loop() {
Kaleidoscope.loop();
}

@ -0,0 +1,6 @@
{
"cpu": {
"fqbn": "keyboardio:virtual:model01",
"port": ""
}
}

@ -0,0 +1,99 @@
/* -*- mode: c++ -*-
* Copyright (C) 2022 Keyboard.io, Inc.
*
* This program is free software: you can redistribute it and/or modify it under
* the terms of the GNU General Public License as published by the Free Software
* Foundation, version 3.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
* FOR A PARTICULAR PURPOSE. See the GNU General Public License for more
* details.
*
* You should have received a copy of the GNU General Public License along with
* this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "testing/setup-googletest.h"
SETUP_GOOGLETEST();
namespace kaleidoscope {
namespace testing {
namespace {
constexpr uint32_t t_0{0};
constexpr uint32_t t_1{1};
constexpr uint32_t t_10{10};
using ::testing::IsEmpty;
class KeyAddrEventQueueLeak : public VirtualDeviceTest {
public:
void assertTimeElapses(uint32_t interval) {
// Record time at start
uint32_t start = Kaleidoscope.millisAtCycleStart();
// Run cycles until the given amount of time has elapsed
sim_.RunForMillis(interval);
// Record time at end
uint32_t end = Kaleidoscope.millisAtCycleStart();
ASSERT_EQ((end - start), interval);
}
};
TEST_F(KeyAddrEventQueueLeak, TimeIsConsistent) {
assertTimeElapses(1);
assertTimeElapses(1);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
assertTimeElapses(10);
}
} // namespace
} // namespace testing
} // namespace kaleidoscope
Loading…
Cancel
Save