Some side tracking fixing undefined behaviour and memory vulnurabilities

meson.build

@@ -1,7 +1,7 @@
 project('ptprnt', 'cpp',
     version: 'v0.1.0-'+run_command('git', 'rev-parse',  '--short', 'HEAD', check: true).stdout().strip(),
     license: 'GPLv3',
-    default_options : ['c_std=c11', 'cpp_std=c++17']    
+    default_options : ['c_std=c11', 'cpp_std=c++17', 'b_sanitize=address,undefined', 'b_lto=true', 'b_lto_mode=thin', 'b_thinlto_cache=true']    
 )
 
 usb_dep = dependency('libusb-1.0')

src/PtouchPrint.cpp

@@ -91,7 +91,7 @@ int PtouchPrint::run() {
             "Found more than one device of the same printer on bus. Currently not supported");
         return -1;
     }
-    printer->attachUsbDevice(devices[0]);
+    printer->attachUsbDevice(std::move(devices[0]));
     auto status = printer->getPrinterStatus();
     spdlog::info("Detected tape width is {}mm", status.tapeWidthMm);
     auto bm = ptprnt::graphics::Bitmap<ptprnt::graphics::ALPHA8>(512, 128);

src/graphics/Monochrome.cpp

@@ -22,6 +22,7 @@
 #include <cmath>
 
 #include <cstdint>
+#include <iostream>
 #include <vector>
 
 namespace ptprnt::graphics {
@@ -41,9 +42,9 @@ std::vector<uint8_t> Monochrome::get() {
 
     unsigned int outIndex = 0;
 
-    for (unsigned int byteNum = 0; byteNum < mPixels.size(); byteNum += 8) {
+    for (unsigned int byteNo = 0; byteNo < mPixels.size(); byteNo += 8) {
-        for (unsigned int bitNo = 0; bitNo < 8; bitNo++) {
+        for (unsigned int bitNo = 0; bitNo <= 7 && (byteNo + bitNo < mPixels.size()); bitNo++) {
-            if (mPixels[byteNum + bitNo] > mThreshhold) {
+            if (mPixels[byteNo + bitNo] > mThreshhold) {
                 outPixels[outIndex] |= (1 << (7 - bitNo));
             } else {
                 outPixels[outIndex] &= ~(1 << (7 - bitNo));

src/libusbwrap/UsbDevice.cpp

@@ -27,12 +27,13 @@
 #include "libusbwrap/interface/IUsbDevice.hpp"
 
 namespace libusbwrap {
-UsbDevice::UsbDevice(libusb_context* ctx, libusb_device* dev) : mLibusbCtx(ctx), mLibusbDev(dev) {
+UsbDevice::UsbDevice(libusb_context* ctx, usbDevice_ptr dev)
+    : mLibusbCtx(ctx), mLibusbDev(std::move(dev)) {
     if (mLibusbCtx == nullptr || mLibusbDev == nullptr) {
         throw std::invalid_argument("ctx or device are nullptr");
     }
 
-    libusb_get_device_descriptor(dev, &mLibusbDevDesc);
+    libusb_get_device_descriptor(mLibusbDev.get(), &mLibusbDevDesc);
 }
 
 UsbDevice::~UsbDevice() {
@@ -44,7 +45,7 @@ UsbDevice::~UsbDevice() {
 }
 
 bool UsbDevice::open() {
-    int openStatus = libusb_open(mLibusbDev, &mLibusbDevHandle);
+    int openStatus = libusb_open(mLibusbDev.get(), &mLibusbDevHandle);
     if (openStatus != 0) {
         mLastError = static_cast<Error>(openStatus);
         return false;
@@ -109,15 +110,15 @@ const usbId UsbDevice::getUsbId() {
 }
 
 const device::Speed UsbDevice::getSpeed() {
-    return static_cast<device::Speed>(libusb_get_device_speed(mLibusbDev));
+    return static_cast<device::Speed>(libusb_get_device_speed(mLibusbDev.get()));
 }
 
 const uint8_t UsbDevice::getBusNumber() {
-    return libusb_get_bus_number(mLibusbDev);
+    return libusb_get_bus_number(mLibusbDev.get());
 }
 
 const uint8_t UsbDevice::getPortNumber() {
-    return libusb_get_port_number(mLibusbDev);
+    return libusb_get_port_number(mLibusbDev.get());
 }
 
 const Error UsbDevice::getLastError() {

src/libusbwrap/UsbDevice.hpp

@@ -27,14 +27,27 @@
 #include "libusbwrap/interface/IUsbDevice.hpp"
 
 namespace libusbwrap {
+
+struct usbDevice_deleter {
+    void operator()(libusb_device* dev_ptr) const {
+        if (nullptr != dev_ptr) {
+            libusb_unref_device(dev_ptr);
+        }
+    }
+};
+
+using usbDevice_ptr = std::unique_ptr<libusb_device, usbDevice_deleter>;
+
 class UsbDevice : public IUsbDevice {
    public:
-    explicit UsbDevice(libusb_context* ctx, libusb_device* dev);
+    explicit UsbDevice(libusb_context* ctx, usbDevice_ptr dev);
     ~UsbDevice() override;
 
     // delete copy ctor and assignment
-    UsbDevice(const UsbDevice&)      = delete;
+    UsbDevice(const UsbDevice&)            = delete;
-    UsbDevice& operator=(UsbDevice&) = delete;
+    UsbDevice& operator=(const UsbDevice&) = delete;
+    UsbDevice(UsbDevice&&)                 = delete;
+    UsbDevice& operator=(UsbDevice&&)      = delete;
 
     bool open() override;
     void close() override;
@@ -58,9 +71,9 @@ class UsbDevice : public IUsbDevice {
 
    private:
     libusb_context* mLibusbCtx{nullptr};
-    libusb_device* mLibusbDev{nullptr};
+    usbDevice_ptr mLibusbDev{nullptr};
     libusb_device_handle* mLibusbDevHandle{nullptr};
-    libusb_device_descriptor mLibusbDevDesc;
+    libusb_device_descriptor mLibusbDevDesc{0};
     std::atomic<bool> mIsOpen = false;
     Error mLastError          = Error::SUCCESS;
 };

src/libusbwrap/UsbDeviceFactory.cpp

@@ -20,9 +20,11 @@
 #include "libusbwrap/UsbDeviceFactory.hpp"
 
 #include <spdlog/spdlog.h>
+#include <sys/types.h>
 
 #include <memory>
 #include <optional>
+#include <vector>
 
 #include "libusb.h"
 #include "libusbwrap/UsbDevice.hpp"
@@ -30,50 +32,53 @@
 
 namespace libusbwrap {
 
-UsbDeviceFactory::~UsbDeviceFactory() {
+UsbDeviceFactory::~UsbDeviceFactory() = default;
-    if (mDeviceListInitialized) {
-        libusb_free_device_list(mLibusbDeviceList, 1);
-    }
-}
 
-std::vector<std::shared_ptr<IUsbDevice>> UsbDeviceFactory::findAllDevices() {
+std::vector<std::unique_ptr<IUsbDevice>> UsbDeviceFactory::findAllDevices() {
     refreshDeviceList();
     return buildMaskedDeviceVector(0x0, 0x0, 0x0, 0x0);
 }
 
-std::vector<std::shared_ptr<IUsbDevice>> UsbDeviceFactory::findDevices(uint16_t vid, uint16_t pid) {
+std::vector<std::unique_ptr<IUsbDevice>> UsbDeviceFactory::findDevices(uint16_t vid, uint16_t pid) {
     refreshDeviceList();
-    return buildMaskedDeviceVector(0xffff, 0xffff, vid, pid);
+    return buildMaskedDeviceVector(LIBUSB_BITMASK_ALL, LIBUSB_BITMASK_ALL, vid, pid);
 }
 
-int UsbDeviceFactory::refreshDeviceList() {
+ssize_t UsbDeviceFactory::refreshDeviceList() {
-    int ret = libusb_get_device_list(mLibusbCtx, &mLibusbDeviceList);
+    libusb_device** list{nullptr};
+
+    ssize_t ret = libusb_get_device_list(mLibusbCtx, &list);
+    mLibusbDeviceList.clear();
     if (ret < 0) {
         spdlog::error("Error enumarating USB devices");
     } else if (ret == 0) {
         spdlog::warn("No USB devices found");
     }
-    mDeviceListInitialized = true;
+
+    for (ssize_t i = 0; i < ret; i++) {
+        mLibusbDeviceList.emplace_back(list[i]);
+    }
+
+    libusb_free_device_list(list, false);
     return ret;
 }
 
-std::vector<std::shared_ptr<IUsbDevice>> UsbDeviceFactory::buildMaskedDeviceVector(uint16_t vidMask,
+std::vector<std::unique_ptr<IUsbDevice>> UsbDeviceFactory::buildMaskedDeviceVector(uint16_t vidMask,
                                                                                    uint16_t pidMask,
                                                                                    uint16_t vid,
                                                                                    uint16_t pid) {
-    std::vector<std::shared_ptr<IUsbDevice>> matchedDevices;
+    std::vector<std::unique_ptr<IUsbDevice>> matchedDevices;
     // see libusb/examples/listdevs.c
-    int i                  = 0;
+    for (auto& currDev : mLibusbDeviceList) {
-    libusb_device* currDev = nullptr;
+        struct libusb_device_descriptor currDevDesc {};
-    while ((currDev = mLibusbDeviceList[i++]) != nullptr) {
+
-        struct libusb_device_descriptor currDevDesc;
+        int ret = libusb_get_device_descriptor(currDev.get(), &currDevDesc);
-        int ret = libusb_get_device_descriptor(currDev, &currDevDesc);
         if (ret < 0) {
             continue;
         }
         if (((currDevDesc.idVendor & vidMask) == vid) &&
             ((currDevDesc.idProduct & pidMask) == pid)) {
-            matchedDevices.push_back(std::make_shared<UsbDevice>(mLibusbCtx, currDev));
+            matchedDevices.push_back(std::make_unique<UsbDevice>(mLibusbCtx, std::move(currDev)));
         }
     }
     return matchedDevices;

src/libusbwrap/UsbDeviceFactory.hpp

@@ -19,9 +19,16 @@
 
 #pragma once
 
+#include <cstdint>
+
+#include "libusb.h"
+#include "libusbwrap/UsbDevice.hpp"
 #include "libusbwrap/interface/IUsbDeviceFactory.hpp"
 
 namespace libusbwrap {
+
+constexpr const uint16_t LIBUSB_BITMASK_ALL = 0xffff;
+
 class UsbDeviceFactory : public IUsbDeviceFactory {
    public:
     UsbDeviceFactory() = default;
@@ -39,7 +46,7 @@ class UsbDeviceFactory : public IUsbDeviceFactory {
      * 
      * @return std::vector<std::shared_ptr<IUsbDevice>> Vector of all detected USB devices
      */
-    std::vector<std::shared_ptr<IUsbDevice>> findAllDevices() override;
+    std::vector<std::unique_ptr<IUsbDevice>> findAllDevices() override;
     /**
      * @brief Gets all devices with certain vid/pid combination. 
      *        If only one device of certain type is connected, vector is usually only one element
@@ -48,17 +55,17 @@ class UsbDeviceFactory : public IUsbDeviceFactory {
      * @param pid ProductId of the devices to find
      * @return std::vector<std::shared_ptr<IUsbDevice>> Vector of detected USB devices based on vid/pid
      */
-    std::vector<std::shared_ptr<IUsbDevice>> findDevices(uint16_t vid, uint16_t pid) override;
+    std::vector<std::unique_ptr<IUsbDevice>> findDevices(uint16_t vid, uint16_t pid) override;
 
    private:
     // methods
-    int refreshDeviceList();
+    ssize_t refreshDeviceList();
-    std::vector<std::shared_ptr<IUsbDevice>> buildMaskedDeviceVector(uint16_t vidMask,
+    std::vector<std::unique_ptr<IUsbDevice>> buildMaskedDeviceVector(uint16_t vidMask,
                                                                      uint16_t pidMask, uint16_t vid,
                                                                      uint16_t pid);
     // members
     libusb_context* mLibusbCtx{nullptr};
-    libusb_device** mLibusbDeviceList{};
+    std::vector<usbDevice_ptr> mLibusbDeviceList{};
     bool mDeviceListInitialized = false;
 };
 }  // namespace libusbwrap

src/libusbwrap/interface/IUsbDeviceFactory.hpp

@@ -10,7 +10,7 @@ namespace libusbwrap {
 class IUsbDeviceFactory {
    public:
     virtual ~IUsbDeviceFactory()                                      = default;
-    virtual std::vector<std::shared_ptr<IUsbDevice>> findAllDevices() = 0;
+    virtual std::vector<std::unique_ptr<IUsbDevice>> findAllDevices() = 0;
-    virtual std::vector<std::shared_ptr<IUsbDevice>> findDevices(uint16_t vid, uint16_t pid) = 0;
+    virtual std::vector<std::unique_ptr<IUsbDevice>> findDevices(uint16_t vid, uint16_t pid) = 0;
 };
 }  // namespace libusbwrap