~ruther/qmk_firmware

a08bcea9983cc97fb2f567c303622495f19a5a1e — Fred Sundvik 9 years ago 3b422d2
Don't accept remote objects with the wrong size

Fixes memory corruption when the crc happens to match, but the size
doesn't.
2 files changed, 59 insertions(+), 14 deletions(-)

M serial_link/protocol/transport.c
M serial_link/tests/transport_tests.c
M serial_link/protocol/transport.c => serial_link/protocol/transport.c +16 -14
@@ 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);
    }
}


M serial_link/tests/transport_tests.c => serial_link/tests/transport_tests.c +43 -0
@@ 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));
}