From 0bb709491baffd69f4f861802f00cf60c77cc2cd Mon Sep 17 00:00:00 2001 From: Vaxry Date: Mon, 22 Jul 2024 13:32:39 +0200 Subject: [PATCH] core: sanitize environment and paths from user data fixes #242 --- src/core/PortalManager.cpp | 2 +- src/helpers/Log.hpp | 5 ++--- src/helpers/MiscFunctions.hpp | 6 +++--- src/portals/Screencopy.cpp | 3 ++- src/portals/Screenshot.cpp | 29 ++++++++++++++++++++--------- src/portals/Screenshot.hpp | 4 ++-- src/shared/ScreencopyShared.cpp | 8 +++++--- src/shared/ScreencopyShared.hpp | 3 +-- 8 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/core/PortalManager.cpp b/src/core/PortalManager.cpp index a31648e..bedc55e 100644 --- a/src/core/PortalManager.cpp +++ b/src/core/PortalManager.cpp @@ -41,7 +41,7 @@ static void handleOutputDone(void* data, struct wl_output* wl_output) { } static void handleOutputMode(void* data, struct wl_output* wl_output, uint32_t flags, int32_t width, int32_t height, int32_t refresh) { - const auto POUTPUT = (SOutput*)data; + const auto POUTPUT = (SOutput*)data; POUTPUT->refreshRate = std::round(refresh / 1000.0); } diff --git a/src/helpers/Log.hpp b/src/helpers/Log.hpp index 97783c4..5fcaaef 100644 --- a/src/helpers/Log.hpp +++ b/src/helpers/Log.hpp @@ -3,8 +3,7 @@ #include #include -enum eLogLevel -{ +enum eLogLevel { TRACE = 0, INFO, LOG, @@ -17,7 +16,7 @@ enum eLogLevel if (!(expr)) { \ Debug::log(CRIT, "\n==========================================================================================\nASSERTION FAILED! \n\n{}\n\nat: line {} in {}", \ std::format(reason, ##__VA_ARGS__), __LINE__, \ - ([]() constexpr->std::string { return std::string(__FILE__).substr(std::string(__FILE__).find_last_of('/') + 1); })().c_str()); \ + ([]() constexpr -> std::string { return std::string(__FILE__).substr(std::string(__FILE__).find_last_of('/') + 1); })().c_str()); \ printf("Assertion failed! See the log in /tmp/hypr/hyprland.log for more info."); \ *((int*)nullptr) = 1; /* so that we crash and get a coredump */ \ } diff --git a/src/helpers/MiscFunctions.hpp b/src/helpers/MiscFunctions.hpp index 4b9764f..f335a83 100644 --- a/src/helpers/MiscFunctions.hpp +++ b/src/helpers/MiscFunctions.hpp @@ -3,6 +3,6 @@ #include std::string execAndGet(const char* cmd); -void addHyprlandNotification(const std::string& icon, float timeMs, const std::string& color, const std::string& message); -bool inShellPath(const std::string& exec); -void sendEmptyDbusMethodReply(sdbus::MethodCall& call, u_int32_t responseCode); \ No newline at end of file +void addHyprlandNotification(const std::string& icon, float timeMs, const std::string& color, const std::string& message); +bool inShellPath(const std::string& exec); +void sendEmptyDbusMethodReply(sdbus::MethodCall& call, u_int32_t responseCode); \ No newline at end of file diff --git a/src/portals/Screencopy.cpp b/src/portals/Screencopy.cpp index 68a5e20..e9fe867 100644 --- a/src/portals/Screencopy.cpp +++ b/src/portals/Screencopy.cpp @@ -689,7 +689,8 @@ void CScreencopyPortal::queueNextShareFrame(CScreencopyPortal::SSession* pSessio Debug::log(TRACE, "[screencopy] set fps {}, frame took {:.2f}ms, ms till next refresh {:.2f}, estimated actual fps: {:.2f}", pSession->sharingData.framerate, FRAMETOOKMS, MSTILNEXTREFRESH, std::clamp(1000.0 / FRAMETOOKMS, 1.0, (double)pSession->sharingData.framerate)); - g_pPortalManager->addTimer({std::clamp(MSTILNEXTREFRESH - 1.0 /* safezone */, 6.0, 1000.0), [pSession]() { g_pPortalManager->m_sPortals.screencopy->startFrameCopy(pSession); }}); + g_pPortalManager->addTimer( + {std::clamp(MSTILNEXTREFRESH - 1.0 /* safezone */, 6.0, 1000.0), [pSession]() { g_pPortalManager->m_sPortals.screencopy->startFrameCopy(pSession); }}); } bool CScreencopyPortal::hasToplevelCapabilities() { return m_sState.toplevel; diff --git a/src/portals/Screenshot.cpp b/src/portals/Screenshot.cpp index ba78dff..8cbf872 100644 --- a/src/portals/Screenshot.cpp +++ b/src/portals/Screenshot.cpp @@ -6,7 +6,9 @@ #include #include -void pickHyprPicker(sdbus::MethodCall& call) { +std::string lastScreenshot; + +void pickHyprPicker(sdbus::MethodCall& call) { const std::string HYPRPICKER_CMD = "hyprpicker --format=rgb --no-fancy"; std::string rgbColor = execAndGet(HYPRPICKER_CMD.c_str()); @@ -68,7 +70,7 @@ void pickSlurp(sdbus::MethodCall& call) { maxValString = maxValString.substr(0, maxValString.find(' ')); uint32_t maxVal = std::stoi(maxValString); - double r, g, b; + double r, g, b; // 1 byte per triplet if (maxVal < 256) { @@ -86,7 +88,7 @@ void pickSlurp(sdbus::MethodCall& call) { b = ((byteString[4] << 8) | byteString[5]) / (maxVal * 1.0); } - auto reply = call.createReply(); + auto reply = call.createReply(); std::unordered_map results; results["color"] = sdbus::Struct(std::tuple{r, g, b}); @@ -133,11 +135,15 @@ void CScreenshotPortal::onScreenshot(sdbus::MethodCall& call) { bool isInteractive = options.count("interactive") && options["interactive"].get() && inShellPath("slurp"); // make screenshot - const std::string HYPR_DIR = "/tmp/hypr/"; - const std::string SNAP_FILE = "xdph_screenshot.png"; - const std::string FILE_PATH = HYPR_DIR + SNAP_FILE; - const std::string SNAP_CMD = "grim " + FILE_PATH; - const std::string SNAP_INTERACTIVE_CMD = "grim -g \"$(slurp)\" " + FILE_PATH; + + const auto RUNTIME_DIR = getenv("XDG_RUNTIME_DIR"); + srand(time(nullptr)); + + const std::string HYPR_DIR = RUNTIME_DIR ? std::string{RUNTIME_DIR} + "/hypr/" : "/tmp/hypr/"; + const std::string SNAP_FILE = std::format("xdph_screenshot_{:x}.png", rand()); // rand() is good enough + const std::string FILE_PATH = HYPR_DIR + SNAP_FILE; + const std::string SNAP_CMD = "grim '" + FILE_PATH + "'"; + const std::string SNAP_INTERACTIVE_CMD = "grim -g \"$(slurp)\" '" + FILE_PATH + "'"; std::unordered_map results; results["uri"] = "file://" + FILE_PATH; @@ -145,6 +151,11 @@ void CScreenshotPortal::onScreenshot(sdbus::MethodCall& call) { std::filesystem::remove(FILE_PATH); std::filesystem::create_directory(HYPR_DIR); + // remove last screenshot. This could cause issues if the app hasn't read the screenshot back yet, but oh well. + if (!lastScreenshot.empty()) + std::filesystem::remove(lastScreenshot); + lastScreenshot = FILE_PATH; + if (isInteractive) execAndGet(SNAP_INTERACTIVE_CMD.c_str()); else @@ -152,7 +163,7 @@ void CScreenshotPortal::onScreenshot(sdbus::MethodCall& call) { uint32_t responseCode = std::filesystem::exists(FILE_PATH) ? 0 : 1; - auto reply = call.createReply(); + auto reply = call.createReply(); reply << responseCode; reply << results; reply.send(); diff --git a/src/portals/Screenshot.hpp b/src/portals/Screenshot.hpp index 3593d5e..b833f62 100644 --- a/src/portals/Screenshot.hpp +++ b/src/portals/Screenshot.hpp @@ -13,6 +13,6 @@ class CScreenshotPortal { private: std::unique_ptr m_pObject; - const std::string INTERFACE_NAME = "org.freedesktop.impl.portal.Screenshot"; - const std::string OBJECT_PATH = "/org/freedesktop/portal/desktop"; + const std::string INTERFACE_NAME = "org.freedesktop.impl.portal.Screenshot"; + const std::string OBJECT_PATH = "/org/freedesktop/portal/desktop"; }; diff --git a/src/shared/ScreencopyShared.cpp b/src/shared/ScreencopyShared.cpp index 0e06675..c3cff1e 100644 --- a/src/shared/ScreencopyShared.cpp +++ b/src/shared/ScreencopyShared.cpp @@ -16,7 +16,7 @@ std::string sanitizeNameForWindowList(const std::string& name) { for (size_t i = 1; i < result.size(); ++i) { if (result[i - 1] == '>' && result[i] == ']') result[i] = ' '; - if (result[i] == '\"') + if (result[i] == '\"' || result[i] == '\'') result[i] = ' '; } return result; @@ -43,8 +43,10 @@ SSelectionData promptForScreencopySelection() { const char* XCURSOR_SIZE = getenv("XCURSOR_SIZE"); const char* HYPRLAND_INSTANCE_SIGNATURE = getenv("HYPRLAND_INSTANCE_SIGNATURE"); - 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", + // 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()); const auto RETVAL = execAndGet(cmd.c_str()); diff --git a/src/shared/ScreencopyShared.hpp b/src/shared/ScreencopyShared.hpp index cc077cf..a71fd23 100644 --- a/src/shared/ScreencopyShared.hpp +++ b/src/shared/ScreencopyShared.hpp @@ -18,8 +18,7 @@ extern "C" { #define XDPH_PWR_BUFFERS_MIN 2 #define XDPH_PWR_ALIGN 16 -enum eSelectionType -{ +enum eSelectionType { TYPE_INVALID = -1, TYPE_OUTPUT = 0, TYPE_WINDOW,