Skip to content

Commit

Permalink
core: move to CProcess from hyprutils
Browse files Browse the repository at this point in the history
this enhances security, avoids calling the shell when invoking the picker
  • Loading branch information
vaxerski committed Nov 15, 2024
1 parent 09b23ce commit 8070f36
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 36 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ pkg_check_modules(
libdrm
gbm
hyprlang>=0.2.0
hyprutils
hyprutils>=0.2.6
hyprwayland-scanner>=0.4.2)

# check whether we can find sdbus-c++ through pkg-config
Expand Down
9 changes: 8 additions & 1 deletion hyprland-share-picker/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ set(CMAKE_CXX_STANDARD_REQUIRED ON)
find_package(QT NAMES Qt6 REQUIRED COMPONENTS Widgets)
find_package(Qt6 REQUIRED COMPONENTS Widgets)

find_package(PkgConfig REQUIRED)
pkg_check_modules(
deps
REQUIRED
IMPORTED_TARGET
hyprutils>=0.2.6)

set(PROJECT_SOURCES main.cpp mainpicker.cpp mainpicker.h mainpicker.ui
elidedbutton.h elidedbutton.cpp)

Expand All @@ -38,7 +45,7 @@ else()
endif()

target_link_libraries(hyprland-share-picker
PRIVATE Qt${QT_VERSION_MAJOR}::Widgets)
PRIVATE Qt${QT_VERSION_MAJOR}::Widgets PkgConfig::deps)

set_target_properties(
hyprland-share-picker
Expand Down
19 changes: 9 additions & 10 deletions hyprland-share-picker/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,21 +15,20 @@
#include <stdexcept>
#include <string>
#include <vector>
#include <hyprutils/os/Process.hpp>
using namespace Hyprutils::OS;

#include "mainpicker.h"
#include "elidedbutton.h"

std::string execAndGet(const char* cmd) {
std::array<char, 128> buffer;
std::string result;
std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd, "r"), pclose);
if (!pipe) {
throw std::runtime_error("popen() failed!");
}
while (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
result += buffer.data();
}
return result;
std::string command = cmd + std::string{" 2>&1"};
CProcess proc("/bin/sh", {"-c", cmd});

if (!proc.runSync())
return "error";

return proc.stdOut();
}

QApplication* pickerPtr = nullptr;
Expand Down
23 changes: 10 additions & 13 deletions src/helpers/MiscFunctions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,17 @@
#include <string>
#include <algorithm>

#include <hyprutils/os/Process.hpp>
using namespace Hyprutils::OS;

std::string execAndGet(const char* cmd) {
Debug::log(LOG, "execAndGet: {}", cmd);

std::array<char, 128> buffer;
std::string result;
const std::unique_ptr<FILE, decltype(&pclose)> pipe(popen(cmd, "r"), pclose);
if (!pipe) {
Debug::log(ERR, "execAndGet: failed in pipe");
return "";
}
while (fgets(buffer.data(), buffer.size(), pipe.get()) != nullptr) {
result += buffer.data();
}
return result;
std::string command = cmd + std::string{" 2>&1"};
CProcess proc("/bin/sh", {"-c", cmd});

if (!proc.runSync())
return "error";

return proc.stdOut();
}

void addHyprlandNotification(const std::string& icon, float timeMs, const std::string& color, const std::string& message) {
Expand Down
35 changes: 24 additions & 11 deletions src/shared/ScreencopyShared.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,15 @@
#include <sys/stat.h>
#include <fcntl.h>

#include <hyprutils/os/Process.hpp>
using namespace Hyprutils::OS;

std::string sanitizeNameForWindowList(const std::string& name) {
std::string result = name;
std::replace(result.begin(), result.end(), '\'', ' ');
std::replace(result.begin(), result.end(), '\"', ' ');
std::replace(result.begin(), result.end(), '$', ' ');
std::replace(result.begin(), result.end(), '`', ' ');
for (size_t i = 1; i < result.size(); ++i) {
if (result[i - 1] == '>' && result[i] == ']')
result[i] = ' ';
Expand Down Expand Up @@ -43,19 +48,28 @@ SSelectionData promptForScreencopySelection() {
static auto* const* PALLOWTOKENBYDEFAULT =
(Hyprlang::INT* const*)g_pPortalManager->m_sConfig.config->getConfigValuePtr("screencopy:allow_token_by_default")->getDataStaticPtr();

// DANGEROUS: we are sending a list of app IDs and titles via env. Make sure it's in 'singlequotes' to avoid something like $(rm -rf /)
// TODO: this is dumb, use a pipe or something.
std::string cmd =
std::format("WAYLAND_DISPLAY='{}' QT_QPA_PLATFORM='wayland' XCURSOR_SIZE='{}' HYPRLAND_INSTANCE_SIGNATURE='{}' XDPH_WINDOW_SHARING_LIST='{}' hyprland-share-picker{} 2>&1",
WAYLAND_DISPLAY ? WAYLAND_DISPLAY : "", XCURSOR_SIZE ? XCURSOR_SIZE : "24", HYPRLAND_INSTANCE_SIGNATURE ? HYPRLAND_INSTANCE_SIGNATURE : "0", buildWindowList(),
(**PALLOWTOKENBYDEFAULT ? " --allow-token" : ""));
std::vector<std::string> args;
if (**PALLOWTOKENBYDEFAULT)
args.emplace_back("--allow-token");

CProcess proc("hyprland-share-picker", args);
proc.addEnv("WAYLAND_DISPLAY", WAYLAND_DISPLAY ? WAYLAND_DISPLAY : "");
proc.addEnv("QT_QPA_PLATFORM", "wayland");
proc.addEnv("XCURSOR_SIZE", XCURSOR_SIZE ? XCURSOR_SIZE : "24");
proc.addEnv("HYPRLAND_INSTANCE_SIGNATURE", HYPRLAND_INSTANCE_SIGNATURE ? HYPRLAND_INSTANCE_SIGNATURE : "0");
proc.addEnv("XDPH_WINDOW_SHARING_LIST", buildWindowList()); // buildWindowList will sanitize any shell stuff in case the picker (qt) does something funky? It shouldn't.

const auto RETVAL = execAndGet(cmd.c_str());
if (!proc.runSync())
return data;

const auto RETVAL = proc.stdOut();
const auto RETVALERR = proc.stdErr();

if (!RETVAL.contains("[SELECTION]")) {
// failed
constexpr const char* QPA_ERR = "qt.qpa.plugin: Could not find the Qt platform plugin";

if (RETVAL.contains("qt.qpa.plugin: Could not find the Qt platform plugin")) {
if (RETVAL.contains(QPA_ERR) || RETVALERR.contains(QPA_ERR)) {
// prompt the user to install qt5-wayland and qt6-wayland
addHyprlandNotification("3", 7000, "0", "[xdph] Could not open the picker: qt5-wayland or qt6-wayland doesn't seem to be installed.");
}
Expand All @@ -71,11 +85,10 @@ SSelectionData promptForScreencopySelection() {
const auto SEL = SELECTION.substr(SELECTION.find_first_of('/') + 1);

for (auto& flag : FLAGS) {
if (flag == 'r') {
if (flag == 'r')
data.allowToken = true;
} else {
else
Debug::log(LOG, "[screencopy] unknown flag from share-picker: {}", flag);
}
}

if (SEL.find("screen:") == 0) {
Expand Down

0 comments on commit 8070f36

Please sign in to comment.