From b482abe3bf62ef46816e81ccdab68bc24f498ce6 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 2 Apr 2020 15:38:39 +0200 Subject: Add test_memory test case This patch adds a test_memory test case that demonstrates new[]/free mismatches with the NK_read_config and NK_get_password_safe_slot_status functions if run with valgrind, see #173. --- CMakeLists.txt | 1 + unittest/test_memory.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 unittest/test_memory.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 1169e94..db8d2fb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -216,6 +216,7 @@ IF (COMPILE_TESTS) unittest/test_HOTP.cc unittest/test1.cc unittest/test_issues.cc + unittest/test_memory.c unittest/test_multiple_devices.cc unittest/test_strdup.cpp unittest/test_safe.cpp diff --git a/unittest/test_memory.c b/unittest/test_memory.c new file mode 100644 index 0000000..34ea7d5 --- /dev/null +++ b/unittest/test_memory.c @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2020 Nitrokey UG + * + * This file is part of libnitrokey. + * + * libnitrokey is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * any later version. + * + * libnitrokey 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 Lesser General Public License + * along with libnitrokey. If not, see . + * + * SPDX-License-Identifier: LGPL-3.0 + */ + +#include +#include "../NK_C_API.h" + +// This test should be run with valgrind to make sure that there are no +// memory leaks in the tested functions: +// valgrind ./test_memory +int main() { + int result = NK_login_auto(); + if (result != 1) + return 1; + + int retry_count = NK_get_admin_retry_count(); + if (retry_count != 3) + return 1; + retry_count = NK_get_user_retry_count(); + if (retry_count != 3) + return 1; + + enum NK_device_model model = NK_get_device_model(); + if (model != NK_PRO && model != NK_STORAGE) + return 1; + + uint8_t *config = NK_read_config(); + if (config == NULL) + return 1; + free(config); + + result = NK_enable_password_safe("123456"); + if (result != 0) + return 1; + + uint8_t *slot_status = NK_get_password_safe_slot_status(); + if (slot_status == NULL) { + return 1; + } + free(slot_status); + + NK_logout(); + + return 0; +} + -- cgit v1.2.1 From 2a7b3f4e2ae09d665f9783030323dfb1a4c5ee9f Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 2 Apr 2020 15:51:25 +0200 Subject: Add missing free functions to C API This patch adds two missing free functions, NK_free_config and NK_free_password_safe_slot_status, to enable memory-safe use of the C API. Fixes #173. --- NK_C_API.cc | 8 ++++++++ NK_C_API.h | 14 ++++++++++++++ unittest/test_memory.c | 4 ++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/NK_C_API.cc b/NK_C_API.cc index 1d3fa3a..0b7f5f7 100644 --- a/NK_C_API.cc +++ b/NK_C_API.cc @@ -226,6 +226,10 @@ extern "C" { }); } + NK_C_API void NK_free_config(uint8_t* config) { + delete[] config; + } + NK_C_API enum NK_device_model NK_get_device_model() { auto m = NitrokeyManager::instance(); @@ -448,6 +452,10 @@ extern "C" { } + NK_C_API void NK_free_password_safe_slot_status(uint8_t* status) { + delete[] status; + } + NK_C_API uint8_t NK_get_user_retry_count() { auto m = NitrokeyManager::instance(); return get_with_result([&]() { diff --git a/NK_C_API.h b/NK_C_API.h index d5c54a3..c4cb448 100644 --- a/NK_C_API.h +++ b/NK_C_API.h @@ -451,6 +451,7 @@ extern "C" { /** * Get currently set config - status of function Numlock/Capslock/Scrollock OTP sending and is enabled PIN protected OTP + * The return value must be freed using NK_free_config. * @see NK_write_config * @return uint8_t general_config[5]: * uint8_t numlock; @@ -462,6 +463,12 @@ extern "C" { */ NK_C_API uint8_t* NK_read_config(); + /** + * Free a value returned by NK_read_config. May be called with a NULL + * argument. + */ + NK_C_API void NK_free_config(uint8_t* config); + //OTP /** @@ -634,10 +641,17 @@ extern "C" { /** * Get password safe slots' status + * The return value must be freed using NK_free_password_safe_slot_status. * @return uint8_t[16] slot statuses - each byte represents one slot with 0 (not programmed) and 1 (programmed) */ NK_C_API uint8_t * NK_get_password_safe_slot_status(); + /** + * Free a value returned by NK_get_password_safe_slot_status. May be + * called with a NULL argument. + */ + NK_C_API void NK_free_password_safe_slot_status(uint8_t* status); + /** * Get password safe slot name * @param slot_number password safe slot number, slot_number<16 diff --git a/unittest/test_memory.c b/unittest/test_memory.c index 34ea7d5..20b11b2 100644 --- a/unittest/test_memory.c +++ b/unittest/test_memory.c @@ -44,7 +44,7 @@ int main() { uint8_t *config = NK_read_config(); if (config == NULL) return 1; - free(config); + NK_free_config(config); result = NK_enable_password_safe("123456"); if (result != 0) @@ -54,7 +54,7 @@ int main() { if (slot_status == NULL) { return 1; } - free(slot_status); + NK_free_password_safe_slot_status(slot_status); NK_logout(); -- cgit v1.2.1