Don't accept remote objects with the wrong size

Fixes memory corruption when the crc happens to match, but the size
doesn't.

diff --git a/serial_link/protocol/transport.c b/serial_link/protocol/transport.c
index efc00e79e0d3eabae059f6136b078992acdc4f46..f418d11ceb68af37034670b953f80d8dcbb33768 100644 (file)
--- 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 02a7a10425bda73257379a0d9780ecbb623d5086..358e1b9fd4502ed54a3f14e4d4de7dde716a8149 100644 (file)
--- 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));
+}