Skip to content

fix: harden backend command validation #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Batuhan4 wants to merge 1 commit into from
Closed

fix: harden backend command validation #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ The installer handles dependency install, user/group creation, DKMS module regis
- `victus-backend.service` launches automatically at boot, stays active 24/7, and keeps Better Auto applied even when no UI client is connected—so fan tweaks persist without needing to open the app.

## Daily Usage
- Launch the GTK app (`victus-control`) or use the CLI client (`test_backend.py`).
- Launch the GTK app (`victus-control`).
- Mode dropdown offers `AUTO`, `Better Auto`, `MANUAL`, `MAX`:
- *Better Auto* is enforced by the background service on each boot, keeps fans in manual PWM, and dynamically adjusts RPMs based on temps/utilisation—ideal for gaming or heavy workloads.
- *Manual* maps slider positions to calibrated RPM steps; fan 2 honours the 10 s offset automatically.
Expand All @@ -51,7 +51,8 @@ meson setup build --prefix=/usr
meson compile -C build
sudo meson install -C build
```
- Smoke test (requires backend running): `python test_backend.py`.
- Run unit tests: `meson test -C build`.
- Manual smoke test (requires backend running): launch `victus-control`, switch modes, and confirm fan RPM and keyboard changes are reflected in the UI.
- The installer fetches `hp-wmi-fan-and-backlight-control`; it’s git-ignored to keep the repo lean.

## Troubleshooting
Expand All @@ -61,7 +62,7 @@ sudo meson install -C build
- **Uninstall**: `sudo systemctl disable --now victus-backend` and `sudo dkms remove hp-wmi-fan-and-backlight-control/0.0.2 --all`.

## Contributing
See `AGENTS.md` for coding style, testing, and PR expectations. Hardware validation notes are welcome in PR descriptions.
Follow the existing Meson/C++ style, run `meson test -C build`, and include hardware validation notes in PR descriptions when you have access to supported laptops.

## License
GPLv3. See `LICENSE` for the full text.
10 changes: 9 additions & 1 deletion backend/meson.build
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
executable('victus-backend',
sources: ['src/fan.cpp', 'src/fan.hpp', 'src/keyboard.cpp','src/keyboard.hpp', 'src/main.cpp', 'src/util.cpp', 'src/util.hpp'],
sources: ['src/fan.cpp', 'src/fan.hpp', 'src/keyboard.cpp', 'src/keyboard.hpp', 'src/main.cpp', 'src/util.cpp', 'src/util.hpp', 'src/validation.cpp', 'src/validation.hpp'],
dependencies: [dependency('threads')],
install: true,
install_dir: get_option('bindir'))

backend_validation_test = executable(
'backend-validation-test',
sources: ['tests/validation_test.cpp', 'src/validation.cpp', 'src/validation.hpp'],
include_directories: include_directories('src'),
install: false)

test('backend-validation', backend_validation_test)

install_data(
'victus-backend.service',
install_dir: '/etc/systemd/system'
Expand Down
170 changes: 99 additions & 71 deletions backend/src/fan.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include <iostream>
#include <fstream>
#include <sstream>
#include <unistd.h>
#include <sys/un.h>
#include <sys/wait.h>
#include <thread>
Expand All @@ -21,6 +22,7 @@

#include "fan.hpp"
#include "util.hpp"
#include "validation.hpp"

static std::atomic<int> fan_thread_generation(0);
static std::atomic<bool> is_reapplying(false);
Expand Down Expand Up @@ -60,6 +62,9 @@ static constexpr std::chrono::seconds kBetterAutoReapply{90};
static constexpr int kBetterAutoCooldownLevel = 5;
static constexpr std::chrono::seconds kBetterAutoCooldown{90};
static constexpr std::chrono::seconds kFanApplyGap{10};
static constexpr const char *kSudoPath = "/usr/bin/sudo";
static constexpr const char *kFanModeHelperPath = "/usr/bin/set-fan-mode.sh";
static constexpr const char *kFanSpeedHelperPath = "/usr/bin/set-fan-speed.sh";

static std::array<std::once_flag, 2> fan_max_once;
static std::array<int, 2> fan_max_cache = kBetterAutoMaxFallback;
Expand Down Expand Up @@ -506,10 +511,49 @@ static bool encode_pwm_mode(const std::string &mode, std::string &encoded)
return false;
}

static int run_helper_command(const std::vector<std::string> &args)
{
if (args.empty()) {
errno = EINVAL;
return -1;
}

std::vector<char *> argv;
argv.reserve(args.size() + 1);
for (const auto &arg : args) {
argv.push_back(const_cast<char *>(arg.c_str()));
}
argv.push_back(nullptr);

pid_t pid = fork();
if (pid < 0) {
return -1;
}

if (pid == 0) {
execv(args.front().c_str(), argv.data());
_exit(127);
}

int status = 0;
while (waitpid(pid, &status, 0) < 0) {
if (errno != EINTR) {
return -1;
}
}

return status;
}

static std::string apply_fan_mode_with_sudo(const std::string &mode)
{
std::string command = "sudo /usr/bin/set-fan-mode.sh " + mode;
int result = system(command.c_str());
std::string encoded_mode;
if (!encode_pwm_mode(mode, encoded_mode)) {
return "ERROR: Invalid fan mode";
}
(void)encoded_mode;

int result = run_helper_command({kSudoPath, kFanModeHelperPath, mode});

if (result == 0) {
return "OK";
Expand Down Expand Up @@ -896,11 +940,18 @@ std::string ensure_better_auto_mode()

std::string get_fan_speed(const std::string &fan_num)
{
auto fan_index = fan_index_from_string(fan_num);
if (!fan_index) {
return "ERROR: Invalid fan number";
}

std::string hwmon_path = find_hwmon_directory("/sys/devices/platform/hp-wmi/hwmon");

if (!hwmon_path.empty())
{
std::ifstream fan_file(hwmon_path + "/fan" + fan_num + "_input");
std::string fan_path =
hwmon_path + "/fan" + std::to_string(*fan_index + 1) + "_input";
std::ifstream fan_file(fan_path);

if (fan_file)
{
Expand Down Expand Up @@ -928,80 +979,46 @@ std::string get_fan_speed(const std::string &fan_num)

std::string set_fan_speed(const std::string &fan_num, const std::string &speed, bool trigger_mode, bool update_cache)
{
int parsed_speed = 0;
bool parsed = false;
try {
parsed_speed = std::stoi(speed);
parsed = true;
} catch (const std::exception &) {
parsed = false;
}

if (parsed) {
size_t index = (fan_num == "2") ? 1 : 0;
int clamped_speed = clamp_to_fan_limits(index, parsed_speed);
if (clamped_speed != parsed_speed) {
std::cout << "set_fan_speed: clamped fan " << fan_num << " target from " << parsed_speed << " to " << clamped_speed << std::endl;
}
std::string clamped_str = std::to_string(clamped_speed);
if (update_cache) {
std::lock_guard<std::mutex> lock(fan_state_mutex);
if (fan_num == "1") {
last_fan1_speed = clamped_str;
} else if (fan_num == "2") {
last_fan2_speed = clamped_str;
}
}

// Update command string if we parsed successfully
std::string command = "sudo /usr/bin/set-fan-speed.sh " + fan_num + " " + clamped_str;

std::unique_lock<std::mutex> apply_lock(fan_apply_mutex);
auto now = std::chrono::steady_clock::now();
if (index == 1 && fan_last_apply[0] != std::chrono::steady_clock::time_point::min()) {
auto elapsed = now - fan_last_apply[0];
if (elapsed < kFanApplyGap) {
auto wait_duration = kFanApplyGap - elapsed;
apply_lock.unlock();
std::this_thread::sleep_for(wait_duration);
apply_lock.lock();
}
}

int result = system(command.c_str());
fan_last_apply[index] = std::chrono::steady_clock::now();
apply_lock.unlock();
auto fan_index = fan_index_from_string(fan_num);
if (!fan_index) {
return "ERROR: Invalid fan number";
}

if (result == 0)
{
// Only trigger fan_mode_trigger if requested and not already reapplying
if (trigger_mode && !is_reapplying.load(std::memory_order_acquire) && get_fan_mode() == "MANUAL") {
fan_mode_trigger("MANUAL");
}
return "OK";
}
else
{
std::cerr << "Failed to execute set-fan-speed.sh for fan " << fan_num << ". Exit code: " << WEXITSTATUS(result) << std::endl;
return "ERROR: Failed to set fan speed";
}
int parsed_speed = 0;
if (!parse_strict_int(speed, &parsed_speed)) {
return "ERROR: Invalid fan speed";
}

// If parsing failed, fall back to original behavior without clamping
size_t index = *fan_index;
int clamped_speed = clamp_to_fan_limits(index, parsed_speed);
if (clamped_speed != parsed_speed) {
std::cout << "set_fan_speed: clamped fan " << fan_num << " target from " << parsed_speed << " to " << clamped_speed << std::endl;
}
std::string clamped_str = std::to_string(clamped_speed);
if (update_cache) {
std::lock_guard<std::mutex> lock(fan_state_mutex);
if (fan_num == "1") {
last_fan1_speed = speed;
} else if (fan_num == "2") {
last_fan2_speed = speed;
if (index == 0) {
last_fan1_speed = clamped_str;
} else {
last_fan2_speed = clamped_str;
}
}

// Construct the command to call the external script with sudo
// The script must be in a location like /usr/bin
std::string command = "sudo /usr/bin/set-fan-speed.sh " + fan_num + " " + speed;
std::unique_lock<std::mutex> apply_lock(fan_apply_mutex);
auto now = std::chrono::steady_clock::now();
if (index == 1 && fan_last_apply[0] != std::chrono::steady_clock::time_point::min()) {
auto elapsed = now - fan_last_apply[0];
if (elapsed < kFanApplyGap) {
auto wait_duration = kFanApplyGap - elapsed;
apply_lock.unlock();
std::this_thread::sleep_for(wait_duration);
apply_lock.lock();
}
}

int result = system(command.c_str());
int result = run_helper_command({kSudoPath, kFanSpeedHelperPath, fan_num, clamped_str});
fan_last_apply[index] = std::chrono::steady_clock::now();
apply_lock.unlock();

if (result == 0)
{
Expand All @@ -1011,9 +1028,20 @@ std::string set_fan_speed(const std::string &fan_num, const std::string &speed,
}
return "OK";
}
else
{
std::cerr << "Failed to execute set-fan-speed.sh for fan " << fan_num << ". Exit code: " << WEXITSTATUS(result) << std::endl;

if (result == -1) {
std::cerr << "Failed to execute set-fan-speed.sh for fan " << fan_num
<< ": " << strerror(errno) << std::endl;
return "ERROR: Failed to set fan speed";
}

if (WIFEXITED(result)) {
std::cerr << "Failed to execute set-fan-speed.sh for fan " << fan_num
<< ". Exit code: " << WEXITSTATUS(result) << std::endl;
} else {
std::cerr << "set-fan-speed.sh terminated abnormally for fan "
<< fan_num << std::endl;
}

return "ERROR: Failed to set fan speed";
}
52 changes: 26 additions & 26 deletions backend/src/keyboard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <vector>

#include "keyboard.hpp"
#include "validation.hpp"

namespace {

Expand Down Expand Up @@ -55,27 +56,6 @@ bool parse_hex_color(const std::string &hex, std::array<int, 3> *rgb) {
return true;
}

bool parse_rgb_triplet(const std::string &color, std::array<int, 3> *rgb) {
std::stringstream ss(color);
int red;
int green;
int blue;
char extra;

if (!(ss >> red >> green >> blue))
return false;

if (ss >> extra)
return false;

if (red < 0 || red > 255 || green < 0 || green > 255 || blue < 0 ||
blue > 255)
return false;

*rgb = {red, green, blue};
return true;
}

std::string hex_to_rgb_string(const std::string &hex) {
std::array<int, 3> rgb;

Expand Down Expand Up @@ -191,8 +171,16 @@ std::string get_keyboard_zone_color(int zone) {
}

std::string set_keyboard_color(const std::string &color) {
std::array<int, 3> rgb_values;
if (!parse_rgb_triplet(color, &rgb_values))
return "ERROR: Invalid RGB color";

std::string canonical_color = std::to_string(rgb_values[0]) + " " +
std::to_string(rgb_values[1]) + " " +
std::to_string(rgb_values[2]);

if (omen_4zone_exists()) {
std::string hex_val = rgb_triplet_to_hex(color);
std::string hex_val = rgb_triplet_to_hex(canonical_color);
if (hex_val.empty())
return "ERROR: Invalid RGB color";

Expand All @@ -207,7 +195,7 @@ std::string set_keyboard_color(const std::string &color) {

std::ofstream rgb(kSingleZoneColorPath);
if (rgb) {
rgb << color;
rgb << canonical_color;
rgb.flush();
if (rgb.fail())
return "ERROR: Failed to write RGB color";
Expand All @@ -222,15 +210,23 @@ std::string set_keyboard_zone_color(int zone, const std::string &color) {
if (zone < 0 || zone >= kFourZoneCount)
return "ERROR: Invalid zone";

std::array<int, 3> rgb_values;
if (!parse_rgb_triplet(color, &rgb_values))
return "ERROR: Invalid RGB color";

std::string canonical_color = std::to_string(rgb_values[0]) + " " +
std::to_string(rgb_values[1]) + " " +
std::to_string(rgb_values[2]);

if (omen_4zone_exists()) {
std::string hex_val = rgb_triplet_to_hex(color);
std::string hex_val = rgb_triplet_to_hex(canonical_color);
if (hex_val.empty())
return "ERROR: Invalid RGB color";

return write_rgb_zone_with_helper(zone, hex_val);
}

return set_keyboard_color(color);
return set_keyboard_color(canonical_color);
}

std::string get_keyboard_brightness() {
Expand All @@ -246,12 +242,16 @@ std::string get_keyboard_brightness() {
}

std::string set_keyboard_brightness(const std::string &value) {
int brightness_value = 0;
if (!parse_bounded_int(value, 0, 255, &brightness_value))
return "ERROR: Invalid keyboard brightness";

if (omen_4zone_exists())
return "OK";

std::ofstream brightness(kSingleZoneBrightnessPath);
if (brightness) {
brightness << value;
brightness << brightness_value;
brightness.flush();
if (brightness.fail())
return "ERROR: Failed to write keyboard brightness";
Expand Down
Loading
Loading