From 7db5b7049ea2181a4252dbe3a027308d59c7d11c Mon Sep 17 00:00:00 2001 From: Christian Hohnstaedt Date: Fri, 13 Oct 2023 23:33:15 +0200 Subject: [PATCH] Close #405: member functions of a1int class have memory leaks. Manage ASN1_INTEGER pointer by QSharedPointer Add AddressSanitizer config to the tests. --- lib/CMakeLists.txt | 2 +- lib/asn1int.cpp | 99 +++++++++++++++++++------------------------- lib/asn1int.h | 11 +++-- lib/test_asn1int.cpp | 32 ++++++++++---- 4 files changed, 72 insertions(+), 72 deletions(-) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 8308c363..280f2e76 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -56,7 +56,7 @@ macro(Test name) add_executable(${name} ${name}.cpp) ExpandSources(${name}) target_link_libraries(${name} PRIVATE - OpenSSL::Crypto ${QT}::Core ${QT}::Test ${QT}::Sql + OpenSSL::Crypto ${QT}::Core ${QT}::Test ${QT}::Sql ${ASAN_LIB} ) add_test(NAME ${name} COMMAND ${name}) set_target_properties(${name} PROPERTIES MACOSX_BUNDLE FALSE) diff --git a/lib/asn1int.cpp b/lib/asn1int.cpp index 63a4b3bf..834bfe30 100644 --- a/lib/asn1int.cpp +++ b/lib/asn1int.cpp @@ -12,65 +12,53 @@ #include #include -ASN1_INTEGER *a1int::dup(const ASN1_INTEGER *a) const +static const QSharedPointer a1init(const ASN1_INTEGER *i) { - // this wrapper casts the const to work around the nonconst - // declared ASN1_STRING_dup (actually it is const - ASN1_INTEGER *r = ASN1_INTEGER_dup((ASN1_INTEGER *)a); + ASN1_INTEGER *a; + if (i) { + a = ASN1_INTEGER_dup(i); + Q_CHECK_PTR(a); + } else { + a = ASN1_INTEGER_new(); + Q_CHECK_PTR(a); + ASN1_INTEGER_set(a, 0); + } + QSharedPointer r(a, ASN1_INTEGER_free); openssl_error(); - if (!r) - r = ASN1_INTEGER_new(); - Q_CHECK_PTR(r); return r; } -a1int::a1int() +a1int::a1int() : in(a1init(nullptr)) { - in = ASN1_INTEGER_new(); - Q_CHECK_PTR(in); - ASN1_INTEGER_set(in, 0); - openssl_error(); } -a1int::a1int(const ASN1_INTEGER *i) +a1int::a1int(const ASN1_INTEGER *i) : in(a1init(i)) { - in = dup(i); } -a1int::a1int(const a1int &a) +a1int::a1int(const a1int &a) : in(a1init(a.get0())) { - in = dup(a.in); } -a1int::a1int(const QString &hex) +a1int::a1int(const QString &hex) : in(a1init(nullptr)) { - in = ASN1_INTEGER_new(); - Q_CHECK_PTR(in); setHex(hex); } -a1int::a1int(long l) +a1int::a1int(long l) : in(a1init(nullptr)) { - in = ASN1_INTEGER_new(); - Q_CHECK_PTR(in); set(l); } -a1int::~a1int() -{ - ASN1_INTEGER_free(in); -} - a1int &a1int::set(const ASN1_INTEGER *i) { - ASN1_INTEGER_free(in); - in = dup(i); + in = a1init(i); return *this; } a1int &a1int::set(long l) { - ASN1_INTEGER_set(in, l); + ASN1_INTEGER_set(in.data(), l); openssl_error(); return *this; } @@ -81,13 +69,12 @@ QString a1int::toQString(int dec) const if (in->length == 0) { return r; } - BIGNUM *bn = ASN1_INTEGER_to_BN(in, NULL); - openssl_error(); - char *res = dec ? BN_bn2dec(bn) : BN_bn2hex(bn); + QSharedPointer bn(ASN1_INTEGER_to_BN(get0(), NULL), BN_free); openssl_error(); + char *res = dec ? BN_bn2dec(bn.data()) : BN_bn2hex(bn.data()); r = res; OPENSSL_free(res); - BN_free(bn); + openssl_error(); return r; } @@ -103,7 +90,7 @@ QString a1int::toDec() const a1int &a1int::setQString(const QString &s, int dec) { - BIGNUM *bn = NULL; + BIGNUM *bn = nullptr; if (s.isEmpty()) { return *this; } @@ -112,9 +99,9 @@ a1int &a1int::setQString(const QString &s, int dec) else BN_hex2bn(&bn, s.toLatin1()); openssl_error(); - BN_to_ASN1_INTEGER(bn, in); - openssl_error(); + BN_to_ASN1_INTEGER(bn, in.data()); BN_free(bn); + openssl_error(); return *this; } @@ -130,41 +117,39 @@ a1int &a1int::setDec(const QString &s) a1int &a1int::setRaw(const unsigned char *data, unsigned len) { - BIGNUM *bn = BN_bin2bn(data, len, NULL); - if (!bn) - openssl_error(); - BN_to_ASN1_INTEGER(bn, in); + QSharedPointer bn(BN_bin2bn(data, len, NULL), BN_free); + openssl_error(); + Q_CHECK_PTR(bn); + BN_to_ASN1_INTEGER(bn.data(), in.data()); openssl_error(); - BN_free(bn); return *this; } ASN1_INTEGER *a1int::get() const { - return dup(in); + return ASN1_INTEGER_dup(get0()); } const ASN1_INTEGER *a1int::get0() const { - return in; + return in.data(); } long a1int::getLong() const { - long l = ASN1_INTEGER_get(in); + long l = ASN1_INTEGER_get(get0()); openssl_error(); return l; } a1int &a1int::operator ++ (void) { - BIGNUM *bn = ASN1_INTEGER_to_BN(in, NULL); + QSharedPointer bn(ASN1_INTEGER_to_BN(get0(), NULL), BN_free); openssl_error(); - BN_add(bn, bn, BN_value_one()); + BN_add(bn.data(), bn.data(), BN_value_one()); openssl_error(); - BN_to_ASN1_INTEGER(bn, in); + BN_to_ASN1_INTEGER(bn.data(), in.data()); openssl_error(); - BN_free(bn); return *this; } @@ -177,35 +162,35 @@ a1int a1int::operator ++ (int) a1int &a1int::operator = (const a1int &a) { - set(a.in); + set(a.get0()); return *this; } a1int &a1int::operator = (long i) { - ASN1_INTEGER_set(in, i); + ASN1_INTEGER_set(in.data(), i); openssl_error(); return *this; } bool a1int::operator > (const a1int &a) const { - return (ASN1_INTEGER_cmp(in, a.in) > 0); + return (ASN1_INTEGER_cmp(get0(), a.get0()) > 0); } bool a1int::operator < (const a1int &a) const { - return (ASN1_INTEGER_cmp(in, a.in) < 0); + return (ASN1_INTEGER_cmp(get0(), a.get0()) < 0); } bool a1int::operator == (const a1int &a) const { - return (ASN1_INTEGER_cmp(in, a.in) == 0); + return (ASN1_INTEGER_cmp(get0(), a.get0()) == 0); } bool a1int::operator != (const a1int &a) const { - return (ASN1_INTEGER_cmp(in, a.in) != 0); + return (ASN1_INTEGER_cmp(get0(), a.get0()) != 0); } a1int::operator QString() const @@ -215,11 +200,11 @@ a1int::operator QString() const QByteArray a1int::i2d() { - return i2d_bytearray(I2D_VOID(i2d_ASN1_INTEGER), in); + return i2d_bytearray(I2D_VOID(i2d_ASN1_INTEGER), get0()); } int a1int::derSize() const { - return i2d_ASN1_INTEGER(in, NULL); + return i2d_ASN1_INTEGER(in.data(), nullptr); } diff --git a/lib/asn1int.h b/lib/asn1int.h index 2948509a..9122fa54 100644 --- a/lib/asn1int.h +++ b/lib/asn1int.h @@ -9,13 +9,13 @@ #define __ASN1INTEGER_H #include +#include #include class a1int { private: - ASN1_INTEGER *in{}; - ASN1_INTEGER *dup(const ASN1_INTEGER *a) const; + QSharedPointer in{}; a1int &setQString(const QString &s, int dec); QString toQString(int dec) const; @@ -25,14 +25,13 @@ class a1int a1int(const a1int &a); a1int(long l); a1int(const QString &hex); - ~a1int(); a1int &set(const ASN1_INTEGER *i); a1int &set(long l); QString toHex() const; QString toDec() const; - a1int &setHex(const QString &s); - a1int &setDec(const QString &s); - a1int &setRaw(const unsigned char *data, unsigned len); + a1int &setHex(const QString &s); + a1int &setDec(const QString &s); + a1int &setRaw(const unsigned char *data, unsigned len); long getLong() const; ASN1_INTEGER *get() const; const ASN1_INTEGER *get0() const; diff --git a/lib/test_asn1int.cpp b/lib/test_asn1int.cpp index c304ff96..df8c92b3 100644 --- a/lib/test_asn1int.cpp +++ b/lib/test_asn1int.cpp @@ -9,15 +9,18 @@ #include #include "asn1int.h" +#include + class test_asn1int: public QObject { Q_OBJECT - private slots: - void constructors(); - void setter(); - void ops(); - void der(); + private slots: + void constructors(); + void setter(); + void ops(); + void der(); + void get(); }; void test_asn1int::constructors() @@ -48,14 +51,27 @@ void test_asn1int::ops() { a1int f = 388; QCOMPARE(f.getLong(), 388); - QCOMPARE(f++.getLong(), 388); + QCOMPARE(f++.getLong(), 388); QCOMPARE((++f).getLong(), 390); QCOMPARE(f.getLong(), 390); a1int s(f); - QCOMPARE(s == f++, true); - QCOMPARE(++s == f, true); + QCOMPARE(s, f++); + QCOMPARE(++s, f); QCOMPARE(++s != f, true); QCOMPARE(s.getLong(), 392); + QCOMPARE(f.getLong(), 391); + QCOMPARE(f < s, true); + QCOMPARE(s > f, true); + QCOMPARE(QString(a1int(0x18929)), "018929"); +} + +void test_asn1int::get() +{ + a1int f(42); + ASN1_INTEGER *g = f.get(); + QCOMPARE(g != f.get0(), true); + QCOMPARE(f.get0(), f.get0()); + ASN1_INTEGER_free(g); } void test_asn1int::der()