summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorFred Sundvik <fsundvik@gmail.com>2016-05-15 10:58:20 +0200
committerFred Sundvik <fsundvik@gmail.com>2016-05-15 10:58:20 +0200
commita08bcea9983cc97fb2f567c303622495f19a5a1e (patch)
treedf4144b4dae3755df0b70a5dfedc4610e45f6ef8
parent3b422d2ac4340ecea6b2fc2f3a855581c737faf8 (diff)
downloadqmk_firmware-a08bcea9983cc97fb2f567c303622495f19a5a1e.tar.gz
qmk_firmware-a08bcea9983cc97fb2f567c303622495f19a5a1e.tar.xz
Don't accept remote objects with the wrong size
Fixes memory corruption when the crc happens to match, but the size doesn't.
-rw-r--r--serial_link/protocol/transport.c30
-rw-r--r--serial_link/tests/transport_tests.c43
2 files changed, 59 insertions, 14 deletions
diff --git a/serial_link/protocol/transport.c b/serial_link/protocol/transport.c
index efc00e79e..f418d11ce 100644
--- a/serial_link/protocol/transport.c
+++ b/serial_link/protocol/transport.c
@@ -73,21 +73,23 @@ void transport_recv_frame(uint8_t from, uint8_t* data, uint16_t size) {
uint8_t id = data[size-1];
if (id < num_remote_objects) {
remote_object_t* obj = remote_objects[id];
- uint8_t* start;
- if (obj->object_type == MASTER_TO_ALL_SLAVES) {
- start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size);
- }
- else if(obj->object_type == SLAVE_TO_MASTER) {
- start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size);
- start += (from - 1) * REMOTE_OBJECT_SIZE(obj->object_size);
- }
- else {
- start = obj->buffer + NUM_SLAVES * LOCAL_OBJECT_SIZE(obj->object_size);
+ if (obj->object_size == size - 1) {
+ uint8_t* start;
+ if (obj->object_type == MASTER_TO_ALL_SLAVES) {
+ start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size);
+ }
+ else if(obj->object_type == SLAVE_TO_MASTER) {
+ start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size);
+ start += (from - 1) * REMOTE_OBJECT_SIZE(obj->object_size);
+ }
+ else {
+ start = obj->buffer + NUM_SLAVES * LOCAL_OBJECT_SIZE(obj->object_size);
+ }
+ triple_buffer_object_t* tb = (triple_buffer_object_t*)start;
+ void* ptr = triple_buffer_begin_write_internal(obj->object_size, tb);
+ memcpy(ptr, data, size - 1);
+ triple_buffer_end_write_internal(tb);
}
- triple_buffer_object_t* tb = (triple_buffer_object_t*)start;
- void* ptr = triple_buffer_begin_write_internal(obj->object_size, tb);
- memcpy(ptr, data, size -1);
- triple_buffer_end_write_internal(tb);
}
}
diff --git a/serial_link/tests/transport_tests.c b/serial_link/tests/transport_tests.c
index 02a7a1042..358e1b9fd 100644
--- a/serial_link/tests/transport_tests.c
+++ b/serial_link/tests/transport_tests.c
@@ -123,3 +123,46 @@ Ensure(Transport, writes_from_master_to_single_slave) {
assert_that(obj2, is_not_equal_to(NULL));
assert_that(obj2->test, is_equal_to(7));
}
+
+Ensure(Transport, ignores_object_with_invalid_id) {
+ update_transport();
+ test_object1_t* obj = begin_write_master_to_single_slave(3);
+ obj->test = 7;
+ expect(signal_data_written);
+ end_write_master_to_single_slave(3);
+ expect(router_send_frame,
+ when(destination, is_equal_to(4)));
+ update_transport();
+ sent_data[sent_data_size - 1] = 44;
+ transport_recv_frame(0, sent_data, sent_data_size);
+ test_object1_t* obj2 = read_master_to_single_slave();
+ assert_that(obj2, is_equal_to(NULL));
+}
+
+Ensure(Transport, ignores_object_with_size_too_small) {
+ update_transport();
+ test_object1_t* obj = begin_write_master_to_slave();
+ obj->test = 7;
+ expect(signal_data_written);
+ end_write_master_to_slave();
+ expect(router_send_frame);
+ update_transport();
+ sent_data[sent_data_size - 2] = 0;
+ transport_recv_frame(0, sent_data, sent_data_size - 1);
+ test_object1_t* obj2 = read_master_to_slave();
+ assert_that(obj2, is_equal_to(NULL));
+}
+
+Ensure(Transport, ignores_object_with_size_too_big) {
+ update_transport();
+ test_object1_t* obj = begin_write_master_to_slave();
+ obj->test = 7;
+ expect(signal_data_written);
+ end_write_master_to_slave();
+ expect(router_send_frame);
+ update_transport();
+ sent_data[sent_data_size + 21] = 0;
+ transport_recv_frame(0, sent_data, sent_data_size + 22);
+ test_object1_t* obj2 = read_master_to_slave();
+ assert_that(obj2, is_equal_to(NULL));
+}