From 4480b08b2dd9aa1362d546d7486452b496a1ea29 Mon Sep 17 00:00:00 2001 From: Michael Richters Date: Thu, 10 Feb 2022 18:30:50 -0600 Subject: [PATCH] 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 --- tests/issues/1107/1107.ino | 71 +++++++++++++++++++++ tests/issues/1107/sketch.json | 6 ++ tests/issues/1107/test/testcase.cpp | 99 +++++++++++++++++++++++++++++ 3 files changed, 176 insertions(+) create mode 100644 tests/issues/1107/1107.ino create mode 100644 tests/issues/1107/sketch.json create mode 100644 tests/issues/1107/test/testcase.cpp diff --git a/tests/issues/1107/1107.ino b/tests/issues/1107/1107.ino new file mode 100644 index 00000000..c3689c05 --- /dev/null +++ b/tests/issues/1107/1107.ino @@ -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 . + */ + +#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 + +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(); +} diff --git a/tests/issues/1107/sketch.json b/tests/issues/1107/sketch.json new file mode 100644 index 00000000..8cc86922 --- /dev/null +++ b/tests/issues/1107/sketch.json @@ -0,0 +1,6 @@ +{ + "cpu": { + "fqbn": "keyboardio:virtual:model01", + "port": "" + } +} diff --git a/tests/issues/1107/test/testcase.cpp b/tests/issues/1107/test/testcase.cpp new file mode 100644 index 00000000..ea4b3e28 --- /dev/null +++ b/tests/issues/1107/test/testcase.cpp @@ -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 . + */ + +#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