Restructure sandbox report for the sake of truncation

microsoft · Sep 21, 2023 · dfbf1cf · dfbf1cf
1 parent 2e5eb5f
commit dfbf1cf
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 6 deletions.
diff --git a/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs b/Public/Src/Engine/Processes/SandboxConnectionLinuxDetours.cs
@@ -384,7 +384,7 @@ private void ProcessBytes((PooledObjectWrapper<byte[]> wrapper, int length) item
                     var message = s_encoding.GetString(item.wrapper.Instance, index: 0, count: item.length).AsSpan().TrimEnd('\n');
 
                     // parse the message, consuming the span field by field. The format is:
-                    //  "%s|%d|%d|%d|%d|%d|%d|%s|%d\n", __progname, getpid(), access, status, explicitLogging, err, opcode, reportPath, isDirectory
+                    //  "%s|%d|%d|%d|%d|%d|%d|%d|%s\n", __progname, getpid(), access, status, explicitLogging, err, opcode, isDirectory, reportPath
                     var restOfMessage = message;
                     _ = nextField(restOfMessage, out restOfMessage);  // ignore progname
                     var pid = AssertInt(nextField(restOfMessage, out restOfMessage));
@@ -393,8 +393,8 @@ private void ProcessBytes((PooledObjectWrapper<byte[]> wrapper, int length) item
                     var explicitlogging = AssertInt(nextField(restOfMessage, out restOfMessage));
                     var err = AssertInt(nextField(restOfMessage, out restOfMessage));
                     var opCode = AssertInt(nextField(restOfMessage, out restOfMessage));
-                    var path = nextField(restOfMessage, out restOfMessage);
                     var isDirectory = AssertInt(nextField(restOfMessage, out restOfMessage));
+                    var path = nextField(restOfMessage, out restOfMessage);
                     Contract.Assert(restOfMessage.IsEmpty);  // We should have reached the end of the message
 
                     // ignore accesses to libDetours.so, because we injected that library

diff --git a/Public/Src/Sandbox/Linux/bxl_observer.cpp b/Public/Src/Sandbox/Linux/bxl_observer.cpp
@@ -362,8 +362,11 @@ bool BxlObserver::SendReport(const AccessReport &report, bool isDebugMessage, bo
         }
         else
         {
-            // The debug message was truncated. Let's crop the debug message so it fits
-            int truncatedSize = PATH_MAX - (numWritten - maxMessageLength);
+            // The report couldn't be fully built for a debug message. Let's crop the message so it fits.
+            // We calculate the maximum size allowed, considering that 'path' is the last component of the
+            // message (plus the \n that ends any report, hence the -1), so it's the last thing 
+            // we tried to write when hitting the size limit.
+            int truncatedSize = PATH_MAX - (numWritten - maxMessageLength) - 1;
             char truncatedMessage[truncatedSize] = {0};
 
             // Let's leave an ending \0

diff --git a/Public/Src/Sandbox/Linux/bxl_observer.hpp b/Public/Src/Sandbox/Linux/bxl_observer.hpp
@@ -328,9 +328,11 @@ class BxlObserver final
     // Builds the report to be sent over the FIFO in the given buffer
     inline int BuildReport(char* buffer, int maxMessageLength, const AccessReport &report, const char *path)
     {
+        // Note: when adding new fields, always leave 'path' as the last component of this message
+        // This is for the sake of the arithmetic when truncating debug messages, where this assumption is made (see SendReport). 
         return snprintf(
-            buffer, maxMessageLength, "%s|%d|%d|%d|%d|%d|%d|%s|%d\n",
-            __progname, report.pid <= 0 ? getpid() : report.pid, report.requestedAccess, report.status, report.reportExplicitly, report.error, report.operation, path, report.isDirectory);
+            buffer, maxMessageLength, "%s|%d|%d|%d|%d|%d|%d|%d|%s\n",
+            __progname, report.pid <= 0 ? getpid() : report.pid, report.requestedAccess, report.status, report.reportExplicitly, report.error, report.operation, report.isDirectory, path);
     }
 
     static BxlObserver *sInstance;