From a46491a97da08e495c92bba8046426678b5564f7 Mon Sep 17 00:00:00 2001
From: Szczepan Zalega <szczepan@nitrokey.com>
Date: Fri, 9 Sep 2016 16:42:31 +0200
Subject: Remove asserts in favor of exceptions or warnings. Test changes in
Python. On possible data truncation return LibraryError(exception) instead of
silently truncating and logging warning
Signed-off-by: Szczepan Zalega <szczepan@nitrokey.com>
---
NitrokeyManager.cc | 10 +++++++---
include/LibraryException.h | 41 +++++++++++++++++++++++++++++++++++++++++
misc.cc | 14 ++++++++------
unittest/test_bindings.py | 24 +++++++++++++++++++++++-
4 files changed, 79 insertions(+), 10 deletions(-)
diff --git a/NitrokeyManager.cc b/NitrokeyManager.cc
index c88f717..d827292 100644
--- a/NitrokeyManager.cc
+++ b/NitrokeyManager.cc
@@ -2,6 +2,7 @@
#include <iostream>
#include "include/NitrokeyManager.h"
#include "include/LibraryException.h"
+#include <algorithm>
namespace nitrokey{
@@ -157,10 +158,13 @@ namespace nitrokey{
return erase_slot(slot_number, temporary_password);
}
- #include <cassert>
template <typename T, typename U>
- void vector_copy(T& dest, std::vector<U> vec){
- assert(sizeof(dest)>=vec.size());
+ void vector_copy(T& dest, std::vector<U> &vec){
+ const size_t d_size = sizeof(dest);
+ if(d_size < vec.size()){
+ throw TargetBufferSmallerThanSource(vec.size(), d_size);
+ }
+ std::fill(dest, dest+d_size, 0);
std::copy(vec.begin(), vec.end(), dest);
}
diff --git a/include/LibraryException.h b/include/LibraryException.h
index 5036320..72891fb 100644
--- a/include/LibraryException.h
+++ b/include/LibraryException.h
@@ -12,6 +12,47 @@ public:
+class TargetBufferSmallerThanSource: public LibraryException {
+public:
+ virtual uint8_t exception_id() override {
+ return 203;
+ }
+
+public:
+ size_t source_size;
+ size_t target_size;
+
+ TargetBufferSmallerThanSource(
+ size_t source_size, size_t target_size
+ ) : source_size(source_size), target_size(target_size) {}
+
+ virtual const char *what() const throw() override {
+ std::string s = " ";
+ auto ts = [](int x){ return std::to_string(x); };
+ std::string msg = std::string("Target buffer size is smaller than source: [source size, buffer size]")
+ +s+ ts(source_size) +s+ ts(target_size);
+ return msg.c_str();
+ }
+
+};
+
+class InvalidHexString : public LibraryException {
+public:
+ virtual uint8_t exception_id() override {
+ return 202;
+ }
+
+public:
+ uint8_t invalid_char;
+
+ InvalidHexString (uint8_t invalid_char) : invalid_char( invalid_char) {}
+
+ virtual const char *what() const throw() override {
+ return "Invalid character in hex string";
+ }
+
+};
+
class InvalidSlotException : public LibraryException {
public:
virtual uint8_t exception_id() override {
diff --git a/misc.cc b/misc.cc
index 5d7c387..9b339cd 100644
--- a/misc.cc
+++ b/misc.cc
@@ -4,25 +4,27 @@
#include "inttypes.h"
#include <cstdlib>
#include <cstring>
-#include <cassert>
+#include "LibraryException.h"
namespace nitrokey {
namespace misc {
std::vector<uint8_t> hex_string_to_byte(const char* hexString){
+ const size_t big_string_size = 256; //arbitrary 'big' number
const size_t s_size = strlen(hexString);
- const size_t d_size = (s_size+1)/2; // add 1 for odd, ignore for even
- assert(s_size%2==0);
- assert(s_size<256); //arbitrary 'big' number
+ const size_t d_size = s_size/2;
+ if (s_size%2!=0 || s_size==0 || s_size>big_string_size){
+ throw InvalidHexString(0);
+ }
auto data = std::vector<uint8_t>(d_size, 0);
char buf[2];
for(int i=0; i<s_size; i++){
char c = hexString[i];
- bool char_from_range = ('0' <= c && c <='9' || 'A' <= c && c <= 'F' || 'a' <= c && c<= 'f');
+ bool char_from_range = (('0' <= c && c <='9') || ('A' <= c && c <= 'F') || ('a' <= c && c<= 'f'));
if (!char_from_range){
- return {};
+ throw InvalidHexString(c);
}
buf[i%2] = c;
if (i%2==1){
diff --git a/unittest/test_bindings.py b/unittest/test_bindings.py
index 377203e..9c266aa 100644
--- a/unittest/test_bindings.py
+++ b/unittest/test_bindings.py
@@ -11,7 +11,8 @@ def to_hex(s):
RFC_SECRET_HR = '12345678901234567890'
-RFC_SECRET = to_hex(RFC_SECRET_HR) #'12345678901234567890'
+RFC_SECRET = to_hex(RFC_SECRET_HR) # '12345678901234567890'
+
# print( repr((RFC_SECRET, RFC_SECRET_, len(RFC_SECRET))) )
@@ -33,6 +34,8 @@ class DeviceErrorCode(Enum):
class LibraryErrors(Enum):
TOO_LONG_STRING = 200
INVALID_SLOT = 201
+ INVALID_HEX_STRING = 202
+ TARGET_BUFFER_SIZE_SMALLER_THAN_SOURCE = 203
@pytest.fixture(scope="module")
@@ -510,3 +513,22 @@ def test_get_serial_number(C):
sn = gs(sn)
assert len(sn) > 0
print(('Serial number of the device: ', sn))
+
+
+@pytest.mark.parametrize("invalid_hex_string",
+ ['text', '00 ', '0xff', 'zzzzzzzzzzzz', 'fff', '', 'f' * 257, 'f' * 258])
+def test_invalid_secret_hex_string_for_OTP_write(C, invalid_hex_string):
+ """
+ Tests for invalid secret hex string during writing to OTP slot. Invalid strings are not hexadecimal number,
+ empty or longer than 255 characters.
+ """
+ assert C.NK_write_hotp_slot(1, 'slot_name', invalid_hex_string, 0, True, False, False, '',
+ DefaultPasswords.ADMIN_TEMP) == LibraryErrors.INVALID_HEX_STRING
+ assert C.NK_write_totp_slot(1, 'python_test', invalid_hex_string, 30, True, False, False, "",
+ DefaultPasswords.ADMIN_TEMP) == LibraryErrors.INVALID_HEX_STRING
+
+
+def test_warning_binary_bigger_than_secret_buffer(C):
+ invalid_hex_string = to_hex('1234567890') * 3
+ assert C.NK_write_hotp_slot(1, 'slot_name', invalid_hex_string, 0, True, False, False, '',
+ DefaultPasswords.ADMIN_TEMP) == LibraryErrors.TARGET_BUFFER_SIZE_SMALLER_THAN_SOURCE
--
cgit v1.2.3