diff options
| -rw-r--r-- | CMakeLists.txt | 1 | ||||
| -rw-r--r-- | NK_C_API.cc | 36 | ||||
| -rw-r--r-- | NK_C_API.h | 32 | ||||
| -rw-r--r-- | NitrokeyManager.cc | 16 | ||||
| -rw-r--r-- | libnitrokey/NitrokeyManager.h | 16 | ||||
| -rw-r--r-- | unittest/test_strdup.cpp | 57 |
6 files changed, 109 insertions, 49 deletions
diff --git a/CMakeLists.txt b/CMakeLists.txt index 06fcfac..3dacb48 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -185,6 +185,7 @@ IF (COMPILE_TESTS) unittest/test1.cc unittest/test_issues.cc unittest/test_multiple_devices.cc + unittest/test_strdup.cpp ) foreach(testsourcefile ${TESTS} ) diff --git a/NK_C_API.cc b/NK_C_API.cc index b245940..8e005b8 100644 --- a/NK_C_API.cc +++ b/NK_C_API.cc @@ -71,7 +71,7 @@ uint8_t * get_with_array_result(T func){ } template <typename T> -const char* get_with_string_result(T func){ +char* get_with_string_result(T func){ NK_last_command_status = 0; try { return func(); @@ -85,7 +85,7 @@ const char* get_with_string_result(T func){ catch (const DeviceCommunicationException &deviceException){ NK_last_command_status = 256-deviceException.getType(); } - return ""; + return strndup("", MAXIMUM_STR_REPLY_LENGTH); } template <typename T> @@ -243,17 +243,17 @@ extern "C" { } - NK_C_API const char * NK_status() { + NK_C_API char * NK_status() { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { string && s = m->get_status_as_string(); - char * rs = strndup(s.c_str(), max_string_field_length); + char * rs = strndup(s.c_str(), MAXIMUM_STR_REPLY_LENGTH); clear_string(s); return rs; }); } - NK_C_API const char * NK_device_serial_number() { + NK_C_API char * NK_device_serial_number() { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { string && s = m->get_serial_number(); @@ -263,11 +263,11 @@ extern "C" { }); } - NK_C_API const char * NK_get_hotp_code(uint8_t slot_number) { + NK_C_API char * NK_get_hotp_code(uint8_t slot_number) { return NK_get_hotp_code_PIN(slot_number, ""); } - NK_C_API const char * NK_get_hotp_code_PIN(uint8_t slot_number, const char *user_temporary_password) { + NK_C_API char * NK_get_hotp_code_PIN(uint8_t slot_number, const char *user_temporary_password) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { string && s = m->get_HOTP_code(slot_number, user_temporary_password); @@ -277,12 +277,12 @@ extern "C" { }); } - NK_C_API const char * NK_get_totp_code(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, + NK_C_API char * NK_get_totp_code(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, uint8_t last_interval) { return NK_get_totp_code_PIN(slot_number, challenge, last_totp_time, last_interval, ""); } - NK_C_API const char * NK_get_totp_code_PIN(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, + NK_C_API char * NK_get_totp_code_PIN(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, uint8_t last_interval, const char *user_temporary_password) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { @@ -327,14 +327,14 @@ extern "C" { }); } - NK_C_API const char* NK_get_totp_slot_name(uint8_t slot_number) { + NK_C_API char* NK_get_totp_slot_name(uint8_t slot_number) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { const auto slot_name = m->get_totp_slot_name(slot_number); return slot_name; }); } - NK_C_API const char* NK_get_hotp_slot_name(uint8_t slot_number) { + NK_C_API char* NK_get_hotp_slot_name(uint8_t slot_number) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { const auto slot_name = m->get_hotp_slot_name(slot_number); @@ -417,20 +417,20 @@ extern "C" { }); } - NK_C_API const char *NK_get_password_safe_slot_name(uint8_t slot_number) { + NK_C_API char *NK_get_password_safe_slot_name(uint8_t slot_number) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { return m->get_password_safe_slot_name(slot_number); }); } - NK_C_API const char *NK_get_password_safe_slot_login(uint8_t slot_number) { + NK_C_API char *NK_get_password_safe_slot_login(uint8_t slot_number) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { return m->get_password_safe_slot_login(slot_number); }); } - NK_C_API const char *NK_get_password_safe_slot_password(uint8_t slot_number) { + NK_C_API char *NK_get_password_safe_slot_password(uint8_t slot_number) { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { return m->get_password_safe_slot_password(slot_number); @@ -589,14 +589,14 @@ extern "C" { }); } - NK_C_API const char* NK_get_status_storage_as_string() { + NK_C_API char* NK_get_status_storage_as_string() { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { return m->get_status_storage_as_string(); }); } - NK_C_API const char* NK_get_SD_usage_data_as_string() { + NK_C_API char* NK_get_SD_usage_data_as_string() { auto m = NitrokeyManager::instance(); return get_with_string_result([&]() { return m->get_SD_usage_data_as_string(); @@ -631,7 +631,7 @@ extern "C" { }); } - NK_C_API const char* NK_list_devices_by_cpuID() { + NK_C_API char* NK_list_devices_by_cpuID() { auto nm = NitrokeyManager::instance(); return get_with_string_result([&]() { auto v = nm->list_devices_by_cpuID(); @@ -640,7 +640,7 @@ extern "C" { res += a+";"; } if (res.size()>0) res.pop_back(); // remove last delimiter char - return strndup(res.c_str(), 8192); //this buffer size sets limit to over 200 devices ID's + return strndup(res.c_str(), MAXIMUM_STR_REPLY_LENGTH); }); } @@ -28,13 +28,15 @@ #ifdef _MSC_VER #define NK_C_API __declspec(dllexport) #else -#define NK_C_API +#define NK_C_API #endif #ifdef __cplusplus extern "C" { #endif + static const int MAXIMUM_STR_REPLY_LENGTH = 8192; + /** * The Nitrokey device models supported by the API. */ @@ -91,13 +93,13 @@ extern "C" { * Return the debug status string. Debug purposes. * @return command processing error code */ - NK_C_API const char * NK_status(); + NK_C_API char * NK_status(); /** * Return the device's serial number string in hex. * @return string device's serial number in hex */ - NK_C_API const char * NK_device_serial_number(); + NK_C_API char * NK_device_serial_number(); /** * Get last command processing status. Useful for commands which returns the results of their own and could not return @@ -183,14 +185,14 @@ extern "C" { * @param slot_number TOTP slot number, slot_number<15 * @return char[20] the name of the slot */ - NK_C_API const char * NK_get_totp_slot_name(uint8_t slot_number); + NK_C_API char * NK_get_totp_slot_name(uint8_t slot_number); /** * * @param slot_number HOTP slot number, slot_number<3 * @return char[20] the name of the slot */ - NK_C_API const char * NK_get_hotp_slot_name(uint8_t slot_number); + NK_C_API char * NK_get_hotp_slot_name(uint8_t slot_number); /** * Erase HOTP slot data from the device @@ -249,7 +251,7 @@ extern "C" { * @param slot_number HOTP slot number, slot_number<3 * @return HOTP code */ - NK_C_API const char * NK_get_hotp_code(uint8_t slot_number); + NK_C_API char * NK_get_hotp_code(uint8_t slot_number); /** * Get HOTP code from the device (PIN protected) @@ -258,7 +260,7 @@ extern "C" { * otherwise should be set to empty string - '' * @return HOTP code */ - NK_C_API const char * NK_get_hotp_code_PIN(uint8_t slot_number, const char *user_temporary_password); + NK_C_API char * NK_get_hotp_code_PIN(uint8_t slot_number, const char *user_temporary_password); /** * Get TOTP code from the device @@ -268,7 +270,7 @@ extern "C" { * @param last_interval last interval * @return TOTP code */ - NK_C_API const char * NK_get_totp_code(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, + NK_C_API char * NK_get_totp_code(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, uint8_t last_interval); /** @@ -281,7 +283,7 @@ extern "C" { * otherwise should be set to empty string - '' * @return TOTP code */ - NK_C_API const char * NK_get_totp_code_PIN(uint8_t slot_number, uint64_t challenge, + NK_C_API char * NK_get_totp_code_PIN(uint8_t slot_number, uint64_t challenge, uint64_t last_totp_time, uint8_t last_interval, const char *user_temporary_password); @@ -343,21 +345,21 @@ extern "C" { * @param slot_number password safe slot number, slot_number<16 * @return slot name */ - NK_C_API const char *NK_get_password_safe_slot_name(uint8_t slot_number); + NK_C_API char *NK_get_password_safe_slot_name(uint8_t slot_number); /** * Get password safe slot login * @param slot_number password safe slot number, slot_number<16 * @return login from the PWS slot */ - NK_C_API const char *NK_get_password_safe_slot_login(uint8_t slot_number); + NK_C_API char *NK_get_password_safe_slot_login(uint8_t slot_number); /** * Get the password safe slot password * @param slot_number password safe slot number, slot_number<16 * @return password from the PWS slot */ - NK_C_API const char *NK_get_password_safe_slot_password(uint8_t slot_number); + NK_C_API char *NK_get_password_safe_slot_password(uint8_t slot_number); /** * Write password safe data to the slot @@ -582,7 +584,7 @@ extern "C" { * Storage only * @return string with devices attributes */ - NK_C_API const char* NK_get_status_storage_as_string(); + NK_C_API char* NK_get_status_storage_as_string(); /** * Get SD card usage attributes as string. @@ -590,7 +592,7 @@ extern "C" { * Storage only * @return string with SD card usage attributes */ - NK_C_API const char* NK_get_SD_usage_data_as_string(); + NK_C_API char* NK_get_SD_usage_data_as_string(); /** * Get progress value of current long operation. @@ -614,7 +616,7 @@ extern "C" { * @example Example of returned data: '00005d19:dacc2cb4_p_0001:0010:02;000037c7:4cf12445_p_0001:000f:02;0001:000c:02' * @return string delimited id's of connected devices */ - NK_C_API const char* NK_list_devices_by_cpuID(); + NK_C_API char* NK_list_devices_by_cpuID(); /** diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc index 2ca183c..085bf78 100644 --- a/NitrokeyManager.cc +++ b/NitrokeyManager.cc @@ -630,12 +630,12 @@ using nitrokey::misc::strcpyT; auto resp = WriteToTOTPSlot::CommandTransaction::run(device, payload); } - const char * NitrokeyManager::get_totp_slot_name(uint8_t slot_number) { + char * NitrokeyManager::get_totp_slot_name(uint8_t slot_number) { if (!is_valid_totp_slot_number(slot_number)) throw InvalidSlotException(slot_number); slot_number = get_internal_slot_number_for_totp(slot_number); return get_slot_name(slot_number); } - const char * NitrokeyManager::get_hotp_slot_name(uint8_t slot_number) { + char * NitrokeyManager::get_hotp_slot_name(uint8_t slot_number) { if (!is_valid_hotp_slot_number(slot_number)) throw InvalidSlotException(slot_number); slot_number = get_internal_slot_number_for_hotp(slot_number); return get_slot_name(slot_number); @@ -643,7 +643,7 @@ using nitrokey::misc::strcpyT; static const int max_string_field_length = 2*1024; //storage's status string is ~1k - const char * NitrokeyManager::get_slot_name(uint8_t slot_number) { + char * NitrokeyManager::get_slot_name(uint8_t slot_number) { auto payload = get_payload<GetSlotName>(); payload.slot_number = slot_number; auto resp = GetSlotName::CommandTransaction::run(device, payload); @@ -749,7 +749,7 @@ using nitrokey::misc::strcpyT; LockDevice::CommandTransaction::run(device); } - const char *NitrokeyManager::get_password_safe_slot_name(uint8_t slot_number) { + char * NitrokeyManager::get_password_safe_slot_name(uint8_t slot_number) { if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number); auto p = get_payload<GetPasswordSafeSlotName>(); p.slot_number = slot_number; @@ -759,7 +759,7 @@ using nitrokey::misc::strcpyT; bool NitrokeyManager::is_valid_password_safe_slot_number(uint8_t slot_number) const { return slot_number < 16; } - const char *NitrokeyManager::get_password_safe_slot_login(uint8_t slot_number) { + char * NitrokeyManager::get_password_safe_slot_login(uint8_t slot_number) { if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number); auto p = get_payload<GetPasswordSafeSlotLogin>(); p.slot_number = slot_number; @@ -767,7 +767,7 @@ using nitrokey::misc::strcpyT; return strndup((const char *) response.data().slot_login, max_string_field_length); } - const char *NitrokeyManager::get_password_safe_slot_password(uint8_t slot_number) { + char * NitrokeyManager::get_password_safe_slot_password(uint8_t slot_number) { if (!is_valid_password_safe_slot_number(slot_number)) throw InvalidSlotException(slot_number); auto p = get_payload<GetPasswordSafeSlotPassword>(); p.slot_number = slot_number; @@ -1059,7 +1059,7 @@ using nitrokey::misc::strcpyT; stick20::ChangeUpdatePassword::CommandTransaction::run(device, p); } - const char * NitrokeyManager::get_status_storage_as_string(){ + char * NitrokeyManager::get_status_storage_as_string(){ auto p = stick20::GetDeviceStatus::CommandTransaction::run(device); return strndup(p.data().dissect().c_str(), max_string_field_length); } @@ -1069,7 +1069,7 @@ using nitrokey::misc::strcpyT; return p.data(); } - const char * NitrokeyManager::get_SD_usage_data_as_string(){ + char * NitrokeyManager::get_SD_usage_data_as_string(){ auto p = stick20::GetSDCardOccupancy::CommandTransaction::run(device); return strndup(p.data().dissect().c_str(), max_string_field_length); } diff --git a/libnitrokey/NitrokeyManager.h b/libnitrokey/NitrokeyManager.h index 1f4cec4..d4630b0 100644 --- a/libnitrokey/NitrokeyManager.h +++ b/libnitrokey/NitrokeyManager.h @@ -93,8 +93,8 @@ char * strndup(const char* str, size_t maxlen); string get_status_as_string(); string get_serial_number(); - const char * get_totp_slot_name(uint8_t slot_number); - const char * get_hotp_slot_name(uint8_t slot_number); + char * get_totp_slot_name(uint8_t slot_number); + char * get_hotp_slot_name(uint8_t slot_number); void change_user_PIN(const char *current_PIN, const char *new_PIN); void change_admin_PIN(const char *current_PIN, const char *new_PIN); @@ -108,9 +108,9 @@ char * strndup(const char* str, size_t maxlen); void lock_device(); - const char *get_password_safe_slot_name(uint8_t slot_number); - const char *get_password_safe_slot_password(uint8_t slot_number); - const char *get_password_safe_slot_login(uint8_t slot_number); + char * get_password_safe_slot_name(uint8_t slot_number); + char * get_password_safe_slot_password(uint8_t slot_number); + char * get_password_safe_slot_login(uint8_t slot_number); void write_password_safe_slot(uint8_t slot_number, const char *slot_name, const char *slot_login, @@ -187,10 +187,10 @@ char * strndup(const char* str, size_t maxlen); void send_startup(uint64_t seconds_from_epoch); - const char * get_status_storage_as_string(); + char * get_status_storage_as_string(); stick20::DeviceConfigurationResponsePacket::ResponsePayload get_status_storage(); - const char *get_SD_usage_data_as_string(); + char * get_SD_usage_data_as_string(); std::pair<uint8_t,uint8_t> get_SD_usage_data(); @@ -227,7 +227,7 @@ char * strndup(const char* str, size_t maxlen); uint8_t get_internal_slot_number_for_hotp(uint8_t slot_number) const; uint8_t get_internal_slot_number_for_totp(uint8_t slot_number) const; bool erase_slot(uint8_t slot_number, const char *temporary_password); - const char * get_slot_name(uint8_t slot_number); + char * get_slot_name(uint8_t slot_number); template <typename ProCommand, PasswordKind StoKind> void change_PIN_general(const char *current_PIN, const char *new_PIN); diff --git a/unittest/test_strdup.cpp b/unittest/test_strdup.cpp new file mode 100644 index 0000000..f980eb9 --- /dev/null +++ b/unittest/test_strdup.cpp @@ -0,0 +1,57 @@ +/* + * Copyright (c) 2015-2018 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 <http://www.gnu.org/licenses/>. + * + * SPDX-License-Identifier: LGPL-3.0 + */ + +// issue: https://github.com/Nitrokey/libnitrokey/issues/110 +// tests according to the issue's author, Robin Krahl (robinkrahl) +// suggested run command: valgrind --tool=memcheck --leak-check=full ./test_strdup + +#include <cstdio> +#include <memory.h> +#include "NK_C_API.h" +#include "catch.hpp" + + +static const int SHORT_STRING_LENGTH = 10; + +TEST_CASE("Test strdup memory free error", "[BASIC]") +{ + NK_set_debug(false); + char *c = NK_status(); /* error --> string literal */ + REQUIRE(c != nullptr); + REQUIRE(strnlen(c, SHORT_STRING_LENGTH) == 0); + puts(c); + free(c); +} + +TEST_CASE("Test strdup memory leak", "[BASIC]") +{ + NK_set_debug(false); + bool connected = NK_login_auto() == 1; + if (!connected) return; + + REQUIRE(connected); + char *c = NK_status(); /* no error --> dynamically allocated */ + REQUIRE(c != nullptr); + REQUIRE(strnlen(c, SHORT_STRING_LENGTH) > 0); + puts(c); + free(c); +} + |