diff --git a/lib/everest/io/src/can/socket_can_handler.cpp b/lib/everest/io/src/can/socket_can_handler.cpp index 3148aa8b17..be7936123e 100644 --- a/lib/everest/io/src/can/socket_can_handler.cpp +++ b/lib/everest/io/src/can/socket_can_handler.cpp @@ -84,6 +84,14 @@ bool socket_can_handler::rx(can_dataset& data) { } bool socket_can_handler::open(std::string const& can_device) { + // IFNAMSIZ is the size of the buffer to write the name to. + // This situation is special concerning null termination, + // The name can occupy the fill buffer. If it does not, nulltermination is necessary + // Since most Linux systems enforce 15 chars as limit plus 1 for nulltermination + // we do the same thing here. + if (can_device.size() >= IFNAMSIZ) { + return false; + } m_can_dev = can_device; return open_device() == 0; } @@ -95,7 +103,11 @@ int socket_can_handler::open_device() { return errno; } struct ifreq ifr; - strcpy(ifr.ifr_name, m_can_dev.c_str()); + memset(&ifr, 0, sizeof(ifr)); + // We know m_can_dev fits because of the check in open(). + // strncpy will copy the string and the null terminator. + // The previous memset handles any trailing bytes in the 16-byte buffer. + strncpy(ifr.ifr_name, m_can_dev.c_str(), IFNAMSIZ); if (ioctl(can_fd, SIOCGIFINDEX, &ifr) < 0) { perror(m_can_dev.c_str()); return errno; diff --git a/modules/HardwareDrivers/PowerSupplies/InfyPower_BEG1K075G/can_driver_acdc/CanDevice.cpp b/modules/HardwareDrivers/PowerSupplies/InfyPower_BEG1K075G/can_driver_acdc/CanDevice.cpp index 91ee801b59..0aabc1826e 100644 --- a/modules/HardwareDrivers/PowerSupplies/InfyPower_BEG1K075G/can_driver_acdc/CanDevice.cpp +++ b/modules/HardwareDrivers/PowerSupplies/InfyPower_BEG1K075G/can_driver_acdc/CanDevice.cpp @@ -29,39 +29,43 @@ CanDevice::~CanDevice() { } bool CanDevice::open_device(const char* dev) { + if (!dev || std::strlen(dev) >= IFNAMSIZ) { + fprintf(stderr, "Interface name is invalid or too long: %s\n", dev ? dev : "NULL"); + return false; + } + if ((can_fd = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) { perror("Socket"); return false; + } - } else { - - // retrieve interface index from interface name - struct ifreq ifr; - strcpy(ifr.ifr_name, dev); - if (ioctl(can_fd, SIOCGIFINDEX, &ifr) < 0) { - perror(dev); - close(can_fd); - return false; - } + // retrieve interface index from interface name + struct ifreq ifr; + std::memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, dev, IFNAMSIZ - 1); + if (ioctl(can_fd, SIOCGIFINDEX, &ifr) < 0) { + perror(dev); + close(can_fd); + return false; + } - // bind to the interface - struct sockaddr_can addr; - memset(&addr, 0, sizeof(addr)); - addr.can_family = AF_CAN; - addr.can_ifindex = ifr.ifr_ifindex; + // bind to the interface + struct sockaddr_can addr; + memset(&addr, 0, sizeof(addr)); + addr.can_family = AF_CAN; + addr.can_ifindex = ifr.ifr_ifindex; - if (bind(can_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { - perror("Bind"); - close(can_fd); - return false; - } + if (bind(can_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + perror("Bind"); + close(can_fd); + return false; + } - // spawn read thread - exit_rx_thread = false; - rx_thread_handle = std::thread(&CanDevice::rx_thread, this); + // spawn read thread + exit_rx_thread = false; + rx_thread_handle = std::thread(&CanDevice::rx_thread, this); - return true; - } + return true; } bool CanDevice::close_device() { diff --git a/modules/HardwareDrivers/PowerSupplies/UUGreenPower_UR1000X0/can_driver_acdc/CanDevice.cpp b/modules/HardwareDrivers/PowerSupplies/UUGreenPower_UR1000X0/can_driver_acdc/CanDevice.cpp index 8ec21fbd5a..52f0bb7b02 100644 --- a/modules/HardwareDrivers/PowerSupplies/UUGreenPower_UR1000X0/can_driver_acdc/CanDevice.cpp +++ b/modules/HardwareDrivers/PowerSupplies/UUGreenPower_UR1000X0/can_driver_acdc/CanDevice.cpp @@ -29,39 +29,43 @@ CanDevice::~CanDevice() { } bool CanDevice::open_device(const char* dev) { + if (!dev || std::strlen(dev) >= IFNAMSIZ) { + fprintf(stderr, "Interface name is invalid or too long: %s\n", dev ? dev : "NULL"); + return false; + } + if ((can_fd = socket(PF_CAN, SOCK_RAW, CAN_RAW)) < 0) { perror("Socket"); return false; + } - } else { - - // retrieve interface index from interface name - struct ifreq ifr; - strcpy(ifr.ifr_name, dev); - if (ioctl(can_fd, SIOCGIFINDEX, &ifr) < 0) { - perror(dev); - close(can_fd); - return false; - } + // retrieve interface index from interface name + struct ifreq ifr; + std::memset(&ifr, 0, sizeof(ifr)); + strncpy(ifr.ifr_name, dev, IFNAMSIZ - 1); + if (ioctl(can_fd, SIOCGIFINDEX, &ifr) < 0) { + perror(dev); + close(can_fd); + return false; + } - // bind to the interface - struct sockaddr_can addr; - memset(&addr, 0, sizeof(addr)); - addr.can_family = AF_CAN; - addr.can_ifindex = ifr.ifr_ifindex; + // bind to the interface + struct sockaddr_can addr; + memset(&addr, 0, sizeof(addr)); + addr.can_family = AF_CAN; + addr.can_ifindex = ifr.ifr_ifindex; - if (bind(can_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { - perror("Bind"); - close(can_fd); - return false; - } + if (bind(can_fd, (struct sockaddr*)&addr, sizeof(addr)) < 0) { + perror("Bind"); + close(can_fd); + return false; + } - // spawn read thread - exit_rx_thread = false; - rx_thread_handle = std::thread(&CanDevice::rx_thread, this); + // spawn read thread + exit_rx_thread = false; + rx_thread_handle = std::thread(&CanDevice::rx_thread, this); - return true; - } + return true; } bool CanDevice::close_device() {