[sandbox] Optimize PolicyDiagnostic::JsonString() memory and type safety

1.Replace unique_ptr<string> with optional<string>
to reduce memory allocations.

2.Use const string& instead of const char*,
introduce LIFETIME_BOUND to clarify the lifetime,
and mark the method with const, mutable, etc. to
ensure the security of the sandbox code.

Bug: None
Change-Id: I1cd4f548e5ea6600a8c84c54860a7347bd7b7f35
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6342732
Commit-Queue: Ho Cheung <hocheung@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1431908}

sandbox/win/src/sandbox.h

@@ -23,9 +23,11 @@
 
 #include <memory>
 #include <optional>
+#include <string>
 #include <string_view>
 #include <vector>
 
+#include "base/compiler_specific.h"
 #include "base/containers/span.h"
 #include "base/win/scoped_process_information.h"
 #include "base/win/windows_types.h"
@@ -236,7 +238,7 @@ class [[clang::lto_visibility_public]] PolicyInfo {
  public:
   // Returns a JSON representation of the policy snapshot.
   // This pointer has the same lifetime as this PolicyInfo object.
-  virtual const char* JsonString() = 0;
+  virtual const std::string& JsonString() const LIFETIME_BOUND = 0;
   virtual ~PolicyInfo() {}
 };
 

sandbox/win/src/sandbox_policy_diagnostic.cc

@@ -336,7 +336,7 @@ base::Value::Dict GetPolicyRules(const std::vector<IpcTag>& ipcs,
 
 // `handle_config` is a set of configuration bools - only output things
 // if they are enabled.
-base::Value::List GetHandlesToClose(HandleCloserConfig& handle_config) {
+base::Value::List GetHandlesToClose(const HandleCloserConfig& handle_config) {
   base::Value::List results;
   if (!handle_config.handle_closer_enabled) {
     return results;
@@ -422,10 +422,10 @@ PolicyDiagnostic::PolicyDiagnostic(PolicyBase* policy) {
 
 PolicyDiagnostic::~PolicyDiagnostic() = default;
 
-const char* PolicyDiagnostic::JsonString() {
+const std::string& PolicyDiagnostic::JsonString() const {
   // Lazily constructs json_string_.
   if (json_string_)
-    return json_string_->c_str();
+    return *json_string_;
 
   base::Value::Dict dict;
   dict.Set(kProcessId, base::strict_cast<double>(process_id_));
@@ -477,8 +477,8 @@ const char* PolicyDiagnostic::JsonString() {
   std::optional<std::string> json_string =
       base::WriteJson(base::Value(std::move(dict)));
   CHECK(json_string);
-  json_string_ = std::make_unique<std::string>(std::move(*json_string));
-  return json_string_->c_str();
+  json_string_ = std::move(json_string);
+  return *json_string_;
 }
 
 }  // namespace sandbox

sandbox/win/src/sandbox_policy_diagnostic.h

@@ -8,10 +8,11 @@
 #include <stddef.h>
 
 #include <memory>
+#include <optional>
 #include <string>
 #include <vector>
 
-#include <optional>
+#include "base/compiler_specific.h"
 #include "base/win/sid.h"
 #include "sandbox/win/src/app_container.h"
 #include "sandbox/win/src/handle_closer.h"
@@ -36,11 +37,11 @@ class PolicyDiagnostic final : public PolicyInfo {
   PolicyDiagnostic& operator=(const PolicyDiagnostic&) = delete;
 
   ~PolicyDiagnostic() override;
-  const char* JsonString() override;
+  const std::string& JsonString() const LIFETIME_BOUND override;
 
  private:
   // |json_string_| is lazily constructed.
-  std::unique_ptr<std::string> json_string_;
+  mutable std::optional<std::string> json_string_;
   uint32_t process_id_;
   TokenLevel lockdown_level_ = USER_LAST;
   JobLevel job_level_ = JobLevel::kUnprotected;