diff options
| author | Fred Sundvik <fsundvik@gmail.com> | 2016-05-15 11:58:20 +0300 |
|---|---|---|
| committer | Fred Sundvik <fsundvik@gmail.com> | 2016-05-15 11:58:20 +0300 |
| commit | a08bcea9983cc97fb2f567c303622495f19a5a1e (patch) | |
| tree | df4144b4dae3755df0b70a5dfedc4610e45f6ef8 | |
| parent | 3b422d2ac4340ecea6b2fc2f3a855581c737faf8 (diff) | |
| download | qmk_firmware-a08bcea9983cc97fb2f567c303622495f19a5a1e.tar.gz qmk_firmware-a08bcea9983cc97fb2f567c303622495f19a5a1e.zip | |
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.c | 30 | ||||
| -rw-r--r-- | serial_link/tests/transport_tests.c | 43 |
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) { | |||
| 73 | uint8_t id = data[size-1]; | 73 | uint8_t id = data[size-1]; |
| 74 | if (id < num_remote_objects) { | 74 | if (id < num_remote_objects) { |
| 75 | remote_object_t* obj = remote_objects[id]; | 75 | remote_object_t* obj = remote_objects[id]; |
| 76 | uint8_t* start; | 76 | if (obj->object_size == size - 1) { |
| 77 | if (obj->object_type == MASTER_TO_ALL_SLAVES) { | 77 | uint8_t* start; |
| 78 | start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size); | 78 | if (obj->object_type == MASTER_TO_ALL_SLAVES) { |
| 79 | } | 79 | start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size); |
| 80 | else if(obj->object_type == SLAVE_TO_MASTER) { | 80 | } |
| 81 | start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size); | 81 | else if(obj->object_type == SLAVE_TO_MASTER) { |
| 82 | start += (from - 1) * REMOTE_OBJECT_SIZE(obj->object_size); | 82 | start = obj->buffer + LOCAL_OBJECT_SIZE(obj->object_size); |
| 83 | } | 83 | start += (from - 1) * REMOTE_OBJECT_SIZE(obj->object_size); |
| 84 | else { | 84 | } |
| 85 | start = obj->buffer + NUM_SLAVES * LOCAL_OBJECT_SIZE(obj->object_size); | 85 | else { |
| 86 | start = obj->buffer + NUM_SLAVES * LOCAL_OBJECT_SIZE(obj->object_size); | ||
| 87 | } | ||
| 88 | triple_buffer_object_t* tb = (triple_buffer_object_t*)start; | ||
| 89 | void* ptr = triple_buffer_begin_write_internal(obj->object_size, tb); | ||
| 90 | memcpy(ptr, data, size - 1); | ||
| 91 | triple_buffer_end_write_internal(tb); | ||
| 86 | } | 92 | } |
| 87 | triple_buffer_object_t* tb = (triple_buffer_object_t*)start; | ||
| 88 | void* ptr = triple_buffer_begin_write_internal(obj->object_size, tb); | ||
| 89 | memcpy(ptr, data, size -1); | ||
| 90 | triple_buffer_end_write_internal(tb); | ||
| 91 | } | 93 | } |
| 92 | } | 94 | } |
| 93 | 95 | ||
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) { | |||
| 123 | assert_that(obj2, is_not_equal_to(NULL)); | 123 | assert_that(obj2, is_not_equal_to(NULL)); |
| 124 | assert_that(obj2->test, is_equal_to(7)); | 124 | assert_that(obj2->test, is_equal_to(7)); |
| 125 | } | 125 | } |
| 126 | |||
| 127 | Ensure(Transport, ignores_object_with_invalid_id) { | ||
| 128 | update_transport(); | ||
| 129 | test_object1_t* obj = begin_write_master_to_single_slave(3); | ||
| 130 | obj->test = 7; | ||
| 131 | expect(signal_data_written); | ||
| 132 | end_write_master_to_single_slave(3); | ||
| 133 | expect(router_send_frame, | ||
| 134 | when(destination, is_equal_to(4))); | ||
| 135 | update_transport(); | ||
| 136 | sent_data[sent_data_size - 1] = 44; | ||
| 137 | transport_recv_frame(0, sent_data, sent_data_size); | ||
| 138 | test_object1_t* obj2 = read_master_to_single_slave(); | ||
| 139 | assert_that(obj2, is_equal_to(NULL)); | ||
| 140 | } | ||
| 141 | |||
| 142 | Ensure(Transport, ignores_object_with_size_too_small) { | ||
| 143 | update_transport(); | ||
| 144 | test_object1_t* obj = begin_write_master_to_slave(); | ||
| 145 | obj->test = 7; | ||
| 146 | expect(signal_data_written); | ||
| 147 | end_write_master_to_slave(); | ||
| 148 | expect(router_send_frame); | ||
| 149 | update_transport(); | ||
| 150 | sent_data[sent_data_size - 2] = 0; | ||
| 151 | transport_recv_frame(0, sent_data, sent_data_size - 1); | ||
| 152 | test_object1_t* obj2 = read_master_to_slave(); | ||
| 153 | assert_that(obj2, is_equal_to(NULL)); | ||
| 154 | } | ||
| 155 | |||
| 156 | Ensure(Transport, ignores_object_with_size_too_big) { | ||
| 157 | update_transport(); | ||
| 158 | test_object1_t* obj = begin_write_master_to_slave(); | ||
| 159 | obj->test = 7; | ||
| 160 | expect(signal_data_written); | ||
| 161 | end_write_master_to_slave(); | ||
| 162 | expect(router_send_frame); | ||
| 163 | update_transport(); | ||
| 164 | sent_data[sent_data_size + 21] = 0; | ||
| 165 | transport_recv_frame(0, sent_data, sent_data_size + 22); | ||
| 166 | test_object1_t* obj2 = read_master_to_slave(); | ||
| 167 | assert_that(obj2, is_equal_to(NULL)); | ||
| 168 | } | ||
