Security & concurrency hardening for the Wi-Fi transfer feature

3ds/include/main.hpp

@@ -31,7 +31,10 @@
 #include "logging.hpp"
 #include <atomic>
 #include <citro2d.h>
+#include <cstdio>
 #include <memory>
+#include <mutex>
+#include <string>
 #include <vector>
 
 inline std::shared_ptr<Screen> g_screen = nullptr;
@@ -54,4 +57,56 @@ inline bool g_transferIsNetwork = false;
 inline u64 g_transferBytesDone  = 0;
 inline u64 g_transferBytesTotal = 0;
 
+// g_transferMode and the byte counters are written by the HTTP server thread
+// (while receiving) and read by the UI thread, so all access must go through
+// these helpers. Concurrent access to the std::string is otherwise undefined
+// behavior, and 64-bit reads tear on the 3DS's 32-bit core.
+inline std::mutex g_transferStatusMutex;
+
+inline void transferSetMode(const std::string& mode)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    g_transferMode = mode;
+}
+
+inline std::string transferGetMode(void)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    return g_transferMode;
+}
+
+inline void transferSetProgress(u64 done, u64 total)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    g_transferBytesDone  = done;
+    g_transferBytesTotal = total;
+}
+
+inline void transferSetDone(u64 done)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    g_transferBytesDone = done;
+}
+
+inline void transferAddDone(u64 delta)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    g_transferBytesDone += delta;
+}
+
+inline void transferGetProgress(u64& done, u64& total)
+{
+    std::lock_guard<std::mutex> lock(g_transferStatusMutex);
+    done  = g_transferBytesDone;
+    total = g_transferBytesTotal;
+}
+
+// Formats a "done / total" byte pair in MB for the transfer progress UI.
+inline std::string transferBytesToMB(u64 done, u64 total)
+{
+    char buf[48];
+    snprintf(buf, sizeof(buf), "%.1f / %.1f MB", (double)done / (1024.0 * 1024.0), (double)total / (1024.0 * 1024.0));
+    return std::string(buf);
+}
+
 #endif

3ds/source/MainScreen.cpp

@@ -227,23 +227,41 @@ void MainScreen::drawTop(void) const
         if (g_isTransferringFile) {
             C2D_DrawRectSolid(0, 0, 0.5f, 400, 240, COLOR_OVERLAY);
 
-            const float size = 0.7f;
-            std::string modeStr;
+            const float size     = 0.7f;
+            const float lineFeed = size * fontGetInfo(NULL)->lineFeed;
+            std::string mode     = transferGetMode();
             if (g_transferIsNetwork) {
-                u64 total = g_transferBytesTotal;
-                u64 done  = g_transferBytesDone;
-                int pct   = total > 0 ? (int)((done * 100) / total) : 0;
-                std::string prefix = g_transferMode.empty() ? "Transferring backup" : g_transferMode;
-                modeStr = StringUtils::format("%s... %d%% (%llu / %llu)", prefix.c_str(), pct, (unsigned long long)done, (unsigned long long)total);
+                // Two centered lines keep the (potentially long) mode label and the
+                // size readout on screen, matching the bottom progress modal.
+                u64 total = 0, done = 0;
+                transferGetProgress(done, total);
+                int pct            = total > 0 ? (int)((done * 100) / total) : 0;
+                std::string prefix = mode.empty() ? "Transferring backup" : mode;
+                std::string line1  = StringUtils::format("%s... %d%%", prefix.c_str(), pct);
+                std::string line2  = transferBytesToMB(done, total);
+
+                float startY = ceilf((240 - 2 * lineFeed) / 2);
+
+                C2D_Text line1Text;
+                C2D_TextParse(&line1Text, dynamicBuf, line1.c_str());
+                C2D_TextOptimize(&line1Text);
+                C2D_DrawText(&line1Text, C2D_WithColor, ceilf((400 - StringUtils::textWidth(line1Text, size)) / 2), startY, 0.9f, size, size,
+                    COLOR_WHITE);
+
+                C2D_Text line2Text;
+                C2D_TextParse(&line2Text, dynamicBuf, line2.c_str());
+                C2D_TextOptimize(&line2Text);
+                C2D_DrawText(&line2Text, C2D_WithColor, ceilf((400 - StringUtils::textWidth(line2Text, size)) / 2), startY + lineFeed, 0.9f, size,
+                    size, COLOR_GREY_LIGHT);
             }
             else {
-                modeStr = (g_transferMode.empty() ? "Copying files" : g_transferMode) + " in progress...";
+                std::string modeStr = (mode.empty() ? "Copying files" : mode) + " in progress...";
+                C2D_Text modeText;
+                C2D_TextParse(&modeText, dynamicBuf, modeStr.c_str());
+                C2D_TextOptimize(&modeText);
+                C2D_DrawText(&modeText, C2D_WithColor, ceilf((400 - StringUtils::textWidth(modeText, size)) / 2), ceilf((240 - lineFeed) / 2), 0.9f,
+                    size, size, COLOR_WHITE);
             }
-            C2D_Text modeText;
-            C2D_TextParse(&modeText, dynamicBuf, modeStr.c_str());
-            C2D_TextOptimize(&modeText);
-            C2D_DrawText(&modeText, C2D_WithColor, ceilf((400 - StringUtils::textWidth(modeText, size)) / 2),
-                ceilf((240 - size * fontGetInfo(NULL)->lineFeed) / 2), 0.9f, size, size, COLOR_WHITE);
         }
     }
 }
@@ -327,10 +345,11 @@ void MainScreen::drawBottom(void) const
             C2D_DrawRectSolid(mx, my, 0.5f, mw, mh, COLOR_BLACK_DARKERR);
             Gui::drawOutline(mx, my, mw, mh, 2, COLOR_PURPLE_LIGHT);
 
-            u64 total = g_transferBytesTotal;
-            u64 done  = g_transferBytesDone;
-            int pct   = total > 0 ? (int)((done * 100) / total) : 0;
-            std::string prefix = g_transferMode.empty() ? "Transferring backup" : g_transferMode;
+            u64 total = 0, done = 0;
+            transferGetProgress(done, total);
+            int pct              = total > 0 ? (int)((done * 100) / total) : 0;
+            std::string mode     = transferGetMode();
+            std::string prefix   = mode.empty() ? "Transferring backup" : mode;
             std::string titleStr = StringUtils::format("%s... %d%%", prefix.c_str(), pct);
 
             C2D_Text titleText;
@@ -339,7 +358,7 @@ void MainScreen::drawBottom(void) const
             C2D_DrawText(
                 &titleText, C2D_WithColor, ceilf(mx + (mw - StringUtils::textWidth(titleText, 0.55f)) / 2), my + 10, 0.5f, 0.55f, 0.55f, COLOR_WHITE);
 
-            std::string bytesStr = StringUtils::format("%llu / %llu", (unsigned long long)done, (unsigned long long)total);
+            std::string bytesStr = transferBytesToMB(done, total);
             C2D_Text bytesText;
             C2D_TextParse(&bytesText, dynamicBuf, bytesStr.c_str());
             C2D_TextOptimize(&bytesText);
@@ -366,7 +385,8 @@ void MainScreen::drawBottom(void) const
             Gui::drawOutline(mx, my, mw, mh, 2, COLOR_PURPLE_LIGHT);
 
             // Title
-            std::string titleStr = (g_transferMode.empty() ? "Copying files" : g_transferMode) + " in progress...";
+            std::string mode     = transferGetMode();
+            std::string titleStr = (mode.empty() ? "Copying files" : mode) + " in progress...";
             C2D_Text titleText;
             C2D_TextParse(&titleText, dynamicBuf, titleStr.c_str());
             C2D_TextOptimize(&titleText);

3ds/source/TransferOverlay.cpp

@@ -164,11 +164,12 @@ void ReceiveOverlay::drawBottom(void) const
 
     int noticeY = 120;
     if (g_transferIsNetwork && g_isTransferringFile) {
-        u64 total = g_transferBytesTotal;
-        u64 done  = g_transferBytesDone;
-        int pct   = total > 0 ? (int)((done * 100) / total) : 0;
-        std::string prefix = g_transferMode.empty() ? "Downloading backup" : g_transferMode;
-        std::string status = StringUtils::format("%s... %d%% (%llu / %llu)", prefix.c_str(), pct, (unsigned long long)done, (unsigned long long)total);
+        u64 total = 0, done = 0;
+        transferGetProgress(done, total);
+        int pct            = total > 0 ? (int)((done * 100) / total) : 0;
+        std::string mode   = transferGetMode();
+        std::string prefix = mode.empty() ? "Downloading backup" : mode;
+        std::string status = StringUtils::format("%s... %d%% (%s)", prefix.c_str(), pct, transferBytesToMB(done, total).c_str());
         C2D_Text statusText;
         C2D_TextParse(&statusText, textBuf, status.c_str());
         C2D_TextOptimize(&statusText);

3ds/source/io.cpp

@@ -256,7 +256,7 @@ std::tuple<bool, Result, std::string> io::backup(size_t index, size_t cellIndex)
 
             g_copyCount    = 0;
             g_copyTotal    = io::countFiles(archive, StringUtils::UTF8toUTF16("/"));
-            g_transferMode = "Backup";
+            transferSetMode("Backup");
 
             res = io::copyDirectory(archive, Archive::sdmc(), StringUtils::UTF8toUTF16("/"), copyPath);
             if (R_FAILED(res)) {
@@ -387,7 +387,7 @@ std::tuple<bool, Result, std::string> io::restore(size_t index, size_t cellIndex
 
             g_copyCount    = 0;
             g_copyTotal    = io::countFiles(Archive::sdmc(), srcPath);
-            g_transferMode = "Restore";
+            transferSetMode("Restore");
 
             res = io::copyDirectory(Archive::sdmc(), archive, srcPath, dstPath);
             if (R_FAILED(res)) {

3ds/source/server.cpp

@@ -31,24 +31,33 @@
 #include <3ds.h>
 #include <cstring>
 #include <map>
+#include <mutex>
 #include <string>
 
 #include <arpa/inet.h>
 #include <atomic>
 #include <cstdlib>
 #include <fcntl.h>
 #include <netinet/in.h>
+#include <poll.h>
 #include <sys/socket.h>
 #include <unistd.h>
 
 namespace {
-    static const int SERVER_PORT   = 8000;
-    std::atomic_flag serverRunning = ATOMIC_FLAG_INIT;
+    static const int SERVER_PORT = 8000;
+    // Hard upper bound on a single request we are willing to buffer in RAM.
+    // The whole request is held in memory, so this caps worst-case allocation
+    // and prevents a malformed/malicious Content-Length from exhausting the heap.
+    static const size_t MAX_REQUEST_SIZE = 32 * 1024 * 1024;
+    std::atomic_flag serverRunning       = ATOMIC_FLAG_INIT;
     s32 serverSocket               = -1;
     std::atomic<bool> serverIsRunning{false};
     std::string serverAddress;
 
     std::map<std::string, Server::HttpHandler> handlers;
+    // handlers is mutated from the main thread (register/unregister on receiver
+    // start/stop) while the network thread looks handlers up, so guard it.
+    std::mutex handlersMutex;
 
     std::string extractPath(const std::string& request)
     {
@@ -82,35 +91,59 @@ namespace {
         return (size_t)strtoul(headers.substr(pos, end - pos).c_str(), nullptr, 10);
     }
 
-    static std::string readRequest(s32 clientSocket)
+    // Per-recv idle timeout. The server is single-threaded, so without a bound a
+    // stalled (or malicious) client would hang the whole receiver indefinitely.
+    // The 3DS SOC layer doesn't support SO_RCVTIMEO, so we gate recv with poll.
+    static const int RECV_TIMEOUT_MS = 15000;
+
+    static std::string readRequest(s32 clientSocket, bool& outTooLarge)
     {
+        outTooLarge = false;
         std::string data;
         data.reserve(4096);
         char buffer[2048];
-        ssize_t received = 0;
-        size_t headerEnd = std::string::npos;
+        ssize_t received     = 0;
+        size_t headerEnd     = std::string::npos;
         size_t contentLength = 0;
         std::string path;
         bool trackTransfer = false;
 
         while (true) {
+            struct pollfd pfd;
+            pfd.fd      = clientSocket;
+            pfd.events  = POLLIN;
+            pfd.revents = 0;
+            int ready   = poll(&pfd, 1, RECV_TIMEOUT_MS);
+            if (ready <= 0) {
+                // Timed out or poll error: abandon this (possibly stalled) request.
+                break;
+            }
             received = recv(clientSocket, buffer, sizeof(buffer), 0);
             if (received <= 0) {
                 break;
             }
             data.append(buffer, received);
 
             if (headerEnd == std::string::npos) {
+                // No header terminator yet: guard against a client that never
+                // sends one (or buries it past the cap) and grows us unbounded.
+                if (data.size() > MAX_REQUEST_SIZE) {
+                    outTooLarge = true;
+                    break;
+                }
                 headerEnd = data.find("\r\n\r\n");
                 if (headerEnd != std::string::npos) {
                     contentLength = parseContentLength(data.substr(0, headerEnd));
-                    path = extractPath(data.substr(0, headerEnd));
+                    path          = extractPath(data.substr(0, headerEnd));
+                    if (contentLength > MAX_REQUEST_SIZE) {
+                        outTooLarge = true;
+                        break;
+                    }
                     if (path == "/transfer/upload") {
                         trackTransfer = true;
                         g_transferIsNetwork = true;
-                        g_transferMode = "Downloading backup";
-                        g_transferBytesTotal = contentLength;
-                        g_transferBytesDone = 0;
+                        transferSetMode("Downloading backup");
+                        transferSetProgress(0, contentLength);
                         g_isTransferringFile = true;
                     }
                     if (contentLength == 0) {
@@ -120,9 +153,11 @@ namespace {
             }
 
             if (headerEnd != std::string::npos) {
+                // contentLength is bounded by MAX_REQUEST_SIZE above, so this read
+                // is bounded too: we stop as soon as the declared body has arrived.
                 size_t totalNeeded = headerEnd + 4 + contentLength;
                 if (trackTransfer && data.size() > headerEnd + 4) {
-                    g_transferBytesDone = data.size() - (headerEnd + 4);
+                    transferSetDone(data.size() - (headerEnd + 4));
                 }
                 if (data.size() >= totalNeeded) {
                     break;
@@ -135,11 +170,32 @@ namespace {
 
     static void handleHttpRequest(s32 clientSocket)
     {
-        std::string request = readRequest(clientSocket);
+        bool tooLarge        = false;
+        std::string request  = readRequest(clientSocket, tooLarge);
+        if (tooLarge) {
+            // Reset any transfer UI state we may have set while reading headers.
+            g_isTransferringFile = false;
+            g_transferIsNetwork  = false;
+            std::string body   = "{\"ok\":false,\"error\":\"Payload too large\"}";
+            std::string header = "HTTP/1.1 413 Payload Too Large\r\nContent-Type: application/json\r\nContent-Length: " +
+                                 std::to_string(body.length()) + "\r\n\r\n";
+            send(clientSocket, header.c_str(), header.length(), 0);
+            send(clientSocket, body.c_str(), body.length(), 0);
+            return;
+        }
         std::string path = extractPath(request);
-        auto it = handlers.find(path);
-        if (it != handlers.end()) {
-            Server::HttpResponse response = it->second(path, request);
+        Server::HttpHandler handler;
+        bool found = false;
+        {
+            std::lock_guard<std::mutex> lock(handlersMutex);
+            auto it = handlers.find(path);
+            if (it != handlers.end()) {
+                handler = it->second;
+                found   = true;
+            }
+        }
+        if (found) {
+            Server::HttpResponse response = handler(path, request);
             std::string header = "HTTP/1.1 " + std::to_string(response.statusCode);
             header += (response.statusCode == 200 ? " OK" : (response.statusCode == 404 ? " Not Found" : " Error"));
             header += "\r\nContent-Type: " + response.contentType;
@@ -188,13 +244,19 @@ namespace {
 
 void Server::registerHandler(const std::string& path, Server::HttpHandler handler)
 {
-    handlers[path] = handler;
+    {
+        std::lock_guard<std::mutex> lock(handlersMutex);
+        handlers[path] = handler;
+    }
     Logging::info("Registered HTTP handler for path {}", path);
 }
 
 void Server::unregisterHandler(const std::string& path)
 {
-    handlers.erase(path);
+    {
+        std::lock_guard<std::mutex> lock(handlersMutex);
+        handlers.erase(path);
+    }
     Logging::info("Unregistered HTTP handler for path {}", path);
 }
 
@@ -254,7 +316,10 @@ void Server::exit()
         serverSocket = -1;
     }
 
-    handlers.clear();
+    {
+        std::lock_guard<std::mutex> lock(handlersMutex);
+        handlers.clear();
+    }
 
     Logging::trace("HTTP server stopped");
 }