core: sanitize environment and paths from user data

fixes #242
hyprwm · Jul 22, 2024 · 0bb7094 · 0bb7094
1 parent b9b97e5
commit 0bb7094
Show file tree

Hide file tree

Showing 8 changed files with 36 additions and 24 deletions.
diff --git 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
@@ -3,8 +3,7 @@
 #include <iostream>
 #include <string>
 
-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
@@ -3,6 +3,6 @@
 #include <sdbus-c++/Message.h>
 
 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);
+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);
diff --git 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
@@ -6,7 +6,9 @@
 #include <regex>
 #include <filesystem>
 
-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<std::string, sdbus::Variant> results;
         results["color"] = sdbus::Struct(std::tuple{r, g, b});
@@ -133,26 +135,35 @@ void CScreenshotPortal::onScreenshot(sdbus::MethodCall& call) {
     bool isInteractive = options.count("interactive") && options["interactive"].get<bool>() && 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<std::string, sdbus::Variant> results;
     results["uri"] = "file://" + FILE_PATH;
 
     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
         execAndGet(SNAP_CMD.c_str());
 
     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
@@ -13,6 +13,6 @@ class CScreenshotPortal {
   private:
     std::unique_ptr<sdbus::IObject> 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
@@ -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
@@ -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,