Skip to content

Commit bb4d4be

Browse files
authored
Merge pull request #17 from jwinarske/jw/flatpak
Fix reliability, security, and correctness issues across codebase
2 parents e6e860f + 76615a4 commit bb4d4be

25 files changed

Lines changed: 283 additions & 77 deletions

CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ message(STATUS "libudev ................ found (version ${UDEV_VERSION})")
5757
add_subdirectory(src/avahi)
5858
add_subdirectory(src/bluez)
5959
add_subdirectory(src/connman)
60+
add_subdirectory(src/flatpak)
6061
add_subdirectory(src/fwupd)
6162
add_subdirectory(src/geoclue2)
6263
add_subdirectory(src/hostname1)

src/avahi/main.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ int main() {
3131
LOG_INFO("Local Service Cookie: {}", server.GetLocalServiceCookie());
3232
LOG_INFO("State: {}", server.GetState());
3333
LOG_INFO("Version: {}", server.GetVersionString());
34-
LOG_INFO("NSSS upport available: {}",
34+
LOG_INFO("NSS Support available: {}",
3535
server.IsNSSSupportAvailable() ? "Yes" : "No");
3636
}
3737

src/bluez/hidraw.hpp

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@
3030
#include <linux/input.h>
3131
#include <linux/types.h>
3232

33-
#include <spdlog/spdlog.h>
33+
#include <cctype>
34+
35+
#include "../utils/logging.h"
3436

3537
#include "hexdump.hpp"
3638

@@ -150,26 +152,26 @@ class Hidraw {
150152
// Use RAII wrapper for automatic cleanup
151153
const UdevPtr udev(udev_new());
152154
if (!udev) {
153-
spdlog::error("Can't create udev");
155+
LOG_ERROR("Can't create udev");
154156
return results; // Safe - RAII cleans up automatically
155157
}
156158

157159
// Use RAII wrapper for enumerating
158160
const UdevEnumeratePtr enumerate(udev_enumerate_new(udev.get()));
159161
if (!enumerate) {
160-
spdlog::error("Can't create udev enumerate");
162+
LOG_ERROR("Can't create udev enumerate");
161163
return results; // Safe - both RAII objects clean up
162164
}
163165

164166
if (const int res = udev_enumerate_add_match_subsystem(enumerate.get(),
165167
sub_system.c_str());
166168
res < 0) {
167-
spdlog::error("Failed to add subsystem match: {}", sub_system);
169+
LOG_ERROR("Failed to add subsystem match: {}", sub_system);
168170
return results; // Safe - RAII cleanup
169171
}
170172

171173
if (const int res = udev_enumerate_scan_devices(enumerate.get()); res < 0) {
172-
spdlog::error("Failed to scan devices");
174+
LOG_ERROR("Failed to scan devices");
173175
return results; // Safe - RAII cleanup
174176
}
175177

@@ -186,7 +188,7 @@ class Hidraw {
186188
// Use RAII wrapper for a device
187189
UdevDevicePtr dev(udev_device_new_from_syspath(udev.get(), path));
188190
if (!dev) {
189-
spdlog::debug("Failed to get device from syspath: {}", path);
191+
LOG_DEBUG("Failed to get device from syspath: {}", path);
190192
continue; // Skip this device, continue with others
191193
}
192194

@@ -201,7 +203,7 @@ class Hidraw {
201203
udev_device_get_property_value(dev.get(), properties_name);
202204
properties[properties_name] = value ? value : "";
203205
if (debug) {
204-
spdlog::debug(" {} = {}", properties_name, value ? value : "");
206+
LOG_DEBUG(" {} = {}", properties_name, value ? value : "");
205207
}
206208
}
207209
}
@@ -262,19 +264,21 @@ class Hidraw {
262264
if (compare_subsystem_device_paths(input_path, hidraw_path)) {
263265
if (hidraw_properties.contains(DEV_NAME)) {
264266
const auto& hidraw_dev_name = hidraw_properties.at(DEV_NAME);
267+
if (!input_properties.contains("UNIQ")) {
268+
continue;
269+
}
265270
auto serial_number = input_properties.at("UNIQ");
266271
const auto dev_key =
267272
create_device_key_from_serial_number(serial_number);
268-
spdlog::info(
269-
"Associated input device path: {} with hidraw device: {}",
270-
input_path, hidraw_dev_name);
273+
LOG_INFO("Associated input device path: {} with hidraw device: {}",
274+
input_path, hidraw_dev_name);
271275
devices_[dev_key] = hidraw_dev_name;
272276
}
273277
}
274278
}
275279
}
276280
for (const auto& value : devices_ | std::views::values) {
277-
spdlog::debug("hidraw device: {}", value);
281+
LOG_DEBUG("hidraw device: {}", value);
278282
}
279283
return true;
280284
}
@@ -327,18 +331,18 @@ class Hidraw {
327331

328332
// Validate that both substrings were found
329333
if (input_pos == std::string::npos) {
330-
spdlog::debug("Path does not contain '/input': {}", input_path);
334+
LOG_DEBUG("Path does not contain '/input': {}", input_path);
331335
return false;
332336
}
333337

334338
if (hidraw_pos == std::string::npos) {
335-
spdlog::debug("Path does not contain '/hidraw': {}", hidraw_path);
339+
LOG_DEBUG("Path does not contain '/hidraw': {}", hidraw_path);
336340
return false;
337341
}
338342

339343
// Ensure the positions are after the prefix
340344
if (input_pos < prefix.size() || hidraw_pos < prefix.size()) {
341-
spdlog::debug("Invalid path structure - subsystem before prefix");
345+
LOG_DEBUG("Invalid path structure - subsystem before prefix");
342346
return false;
343347
}
344348

@@ -366,7 +370,7 @@ class Hidraw {
366370

367371
// Convert to uppercase
368372
std::ranges::transform(converted_serial, converted_serial.begin(),
369-
::toupper);
373+
[](unsigned char c) { return std::toupper(c); });
370374

371375
// Replace ':' with '_'
372376
std::ranges::replace(converted_serial, ':', '_');

src/bluez/horipad_steam/horipad_steam.cc

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,10 @@ HoripadSteam::HoripadSteam(sdbus::IConnection& connection)
4545
sub_system ? sub_system : "");
4646
if (std::strcmp(sub_system, "hidraw") == 0) {
4747
if (std::strcmp(action, "remove") == 0) {
48-
input_reader_->stop();
49-
input_reader_.reset();
48+
if (input_reader_) {
49+
input_reader_->stop();
50+
input_reader_.reset();
51+
}
5052
}
5153
if (!get_hidraw_devices(input_match_params_bt)) {
5254
get_hidraw_devices(input_match_params_usb);

src/bluez/horipad_steam/input_reader.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ InputReader::Task InputReader::read_input() {
9494
}
9595
LOG_INFO("Report Descriptor Size: {}", desc_size);
9696

97+
if (desc_size < 0 ||
98+
static_cast<unsigned>(desc_size) > HID_MAX_DESCRIPTOR_SIZE) {
99+
LOG_ERROR("Invalid report descriptor size: {}", desc_size);
100+
break;
101+
}
102+
97103
// Report Descriptor
98104
hidraw_report_descriptor rpt_desc{};
99105
rpt_desc.size = desc_size;

src/bluez/horipad_steam/input_reader.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
#ifndef SRC_BLUEZ_XBOX_CONTROLLER_INPUT_READER_HPP_
1616
#define SRC_BLUEZ_XBOX_CONTROLLER_INPUT_READER_HPP_
1717

18-
#include "horipad_stream_01ab_0196.h"
19-
2018
#include <atomic>
2119
#include <coroutine>
2220

src/bluez/ps5_dual_sense/dual_sense.cc

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#include "dual_sense.h"
1616

17+
#include <cctype>
18+
1719
#include "../../utils/property_utils.h"
1820
#include "../../utils/resource_limits.h"
1921

@@ -42,8 +44,10 @@ DualSense::DualSense(sdbus::IConnection& connection)
4244
sub_system ? sub_system : "");
4345
if (std::strcmp(sub_system, "hidraw") == 0) {
4446
if (std::strcmp(action, "remove") == 0) {
45-
input_reader_->stop();
46-
input_reader_.reset();
47+
if (input_reader_) {
48+
input_reader_->stop();
49+
input_reader_.reset();
50+
}
4751
}
4852
if (!get_hidraw_devices(input_match_bt)) {
4953
get_hidraw_devices(input_match_usb);
@@ -250,7 +254,8 @@ std::string DualSense::convert_mac_to_path(const std::string& mac_address) {
250254
"/org/freedesktop/UPower/devices/battery_ps_controller_battery_";
251255
std::string converted_mac = mac_address;
252256
std::ranges::replace(converted_mac, ':', 'o');
253-
std::ranges::transform(converted_mac, converted_mac.begin(), ::tolower);
257+
std::ranges::transform(converted_mac, converted_mac.begin(),
258+
[](unsigned char c) { return std::tolower(c); });
254259
result += converted_mac;
255260
return result;
256261
}

src/bluez/ps5_dual_sense/input_reader.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ InputReader::Task InputReader::read_input() {
9595
}
9696
LOG_INFO("Report Descriptor Size: {}", desc_size);
9797

98+
if (desc_size < 0 ||
99+
static_cast<unsigned>(desc_size) > HID_MAX_DESCRIPTOR_SIZE) {
100+
LOG_ERROR("Invalid report descriptor size: {}", desc_size);
101+
break;
102+
}
103+
98104
// Report Descriptor
99105
hidraw_report_descriptor rpt_desc{};
100106
rpt_desc.size = desc_size;

src/bluez/udev_monitor.hpp

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
#include <sys/epoll.h>
3030
#include <unistd.h>
3131

32-
#include <spdlog/spdlog.h>
32+
#include "../utils/logging.h"
3333

3434
// ---------------------------------------------------------------------------
3535
// RAII wrappers for libudev handles
@@ -72,8 +72,7 @@ class UdevMonitor {
7272
callback)
7373
: sub_systems_(std::move(sub_systems)), callback_(callback) {
7474
if (pipe(pipe_fds_) == -1) {
75-
spdlog::error("Failed to create pipe: {} ({})", std::strerror(errno),
76-
errno);
75+
LOG_ERROR("Failed to create pipe: {} ({})", std::strerror(errno), errno);
7776
pipe_fds_[0] = -1;
7877
pipe_fds_[1] = -1;
7978
return;
@@ -125,8 +124,8 @@ class UdevMonitor {
125124
// Signal the worker thread to exit
126125
if (pipe_fds_[1] != -1) {
127126
if (const ssize_t wrote = write(pipe_fds_[1], "x", 1); wrote == -1) {
128-
spdlog::error("Failed to write to stop pipe: {} ({})",
129-
std::strerror(errno), errno);
127+
LOG_ERROR("Failed to write to stop pipe: {} ({})", std::strerror(errno),
128+
errno);
130129
}
131130
}
132131
}
@@ -142,15 +141,15 @@ class UdevMonitor {
142141
// RAII: udev context — automatically udev_unref'd on scope exit
143142
const UdevPtr udev(udev_new());
144143
if (!udev) {
145-
spdlog::error("Failed to create udev context");
144+
LOG_ERROR("Failed to create udev context");
146145
is_running_ = false;
147146
return;
148147
}
149148

150149
// RAII: udev monitor — automatically udev_monitor_unref'd on scope exit
151150
const UdevMonitorPtr mon(udev_monitor_new_from_netlink(udev.get(), "udev"));
152151
if (!mon) {
153-
spdlog::error("Failed to create udev monitor");
152+
LOG_ERROR("Failed to create udev monitor");
154153
is_running_ = false;
155154
return;
156155
}
@@ -159,7 +158,7 @@ class UdevMonitor {
159158
if (int res = udev_monitor_filter_add_match_subsystem_devtype(
160159
mon.get(), sub_system.c_str(), nullptr);
161160
res != 0) {
162-
spdlog::error(
161+
LOG_ERROR(
163162
"udev_monitor_filter_add_match_subsystem_devtype failed on {} = {}",
164163
sub_system, res);
165164
}
@@ -170,8 +169,7 @@ class UdevMonitor {
170169
// RAII: epoll fd — automatically closed on scope exit
171170
const EpollFd epoll_fd(epoll_create1(0));
172171
if (!epoll_fd.valid()) {
173-
spdlog::error("Failed to create epoll: {} ({})", std::strerror(errno),
174-
errno);
172+
LOG_ERROR("Failed to create epoll: {} ({})", std::strerror(errno), errno);
175173
is_running_ = false;
176174
return;
177175
}
@@ -180,16 +178,16 @@ class UdevMonitor {
180178
ev.events = EPOLLIN;
181179
ev.data.fd = fd;
182180
if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, fd, &ev) == -1) {
183-
spdlog::error("Failed to add udev fd to epoll: {} ({})",
184-
std::strerror(errno), errno);
181+
LOG_ERROR("Failed to add udev fd to epoll: {} ({})", std::strerror(errno),
182+
errno);
185183
is_running_ = false;
186184
return;
187185
}
188186

189187
ev.data.fd = pipe_fds_[0];
190188
if (epoll_ctl(epoll_fd.get(), EPOLL_CTL_ADD, pipe_fds_[0], &ev) == -1) {
191-
spdlog::error("Failed to add pipe fd to epoll: {} ({})",
192-
std::strerror(errno), errno);
189+
LOG_ERROR("Failed to add pipe fd to epoll: {} ({})", std::strerror(errno),
190+
errno);
193191
is_running_ = false;
194192
return;
195193
}
@@ -202,14 +200,13 @@ class UdevMonitor {
202200
if (errno == EINTR) {
203201
continue; // Interrupted by signal, retry
204202
}
205-
spdlog::error("epoll_wait failed: {} ({})", std::strerror(errno),
206-
errno);
203+
LOG_ERROR("epoll_wait failed: {} ({})", std::strerror(errno), errno);
207204
break;
208205
}
209206

210207
for (int n = 0; n < triggered_event_count; ++n) {
211208
if (events[n].data.fd == pipe_fds_[0]) {
212-
spdlog::info("Pipe was written to, exiting the loop");
209+
LOG_INFO("Pipe was written to, exiting the loop");
213210
is_running_ = false;
214211
break;
215212
}
@@ -225,7 +222,7 @@ class UdevMonitor {
225222
if (action && subsystem) {
226223
callback_(action, devnode, subsystem);
227224
} else {
228-
spdlog::debug(
225+
LOG_DEBUG(
229226
"Skipping callback for device with missing properties: "
230227
"action={}, devnode={}, subsystem={}",
231228
action ? action : "null", devnode ? devnode : "null",
@@ -239,7 +236,7 @@ class UdevMonitor {
239236
}
240237

241238
// All resources (epoll_fd, mon, udev) are released by their destructors.
242-
spdlog::debug("UdevMonitor worker thread exiting");
239+
LOG_DEBUG("UdevMonitor worker thread exiting");
243240
}
244241
};
245242

src/bluez/xbox_controller/input_reader.cc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,12 @@ InputReader::Task InputReader::read_input() {
9595
}
9696
LOG_INFO("Report Descriptor Size: {}", desc_size);
9797

98+
if (desc_size < 0 ||
99+
static_cast<unsigned>(desc_size) > HID_MAX_DESCRIPTOR_SIZE) {
100+
LOG_ERROR("Invalid report descriptor size: {}", desc_size);
101+
break;
102+
}
103+
98104
// Report Descriptor
99105
hidraw_report_descriptor rpt_desc{};
100106
rpt_desc.size = desc_size;

0 commit comments

Comments
 (0)