From 020dc771b20687dde989d5b260d84907448671fe Mon Sep 17 00:00:00 2001
From: Henner Zeller <h.zeller@acm.org>
Date: Tue, 11 Mar 2025 10:14:44 -0700
Subject: [PATCH] Fix potential memory access issues

  * std::string_view::data() does not necessarily have a NUL
    terminated string. So either convert to std::string() first
    or have function parameters use a const char* to begin with
    if that is what the caller provides.
  * subtle: the absl cleanup got a _copy_ of the initial allocated
    buffer pointer, but getline() will modify possibly that value as it
    might reallocate. So call the cleanup with a reference.

Not an access issue, just simplification:
  * in getting the env var, no need to convert to string_view first,
    then back to string just to have a nul-terminated string.
    We call that function with a const char* anyway.
---
 fpga/assembler.cc          | 7 +++----
 fpga/database-parsers.cc   | 8 ++++----
 fpga/memory-mapped-file.cc | 3 +--
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/fpga/assembler.cc b/fpga/assembler.cc
index e42d9e8..8614605 100644
--- a/fpga/assembler.cc
+++ b/fpga/assembler.cc
@@ -268,7 +268,7 @@ static absl::Status AssembleFrames(FILE *input_stream, fpga::PartDatabase &db,
   // Parse fasm.
   size_t buf_size = 8192;
   char *buffer = (char *)malloc(buf_size);
-  const absl::Cleanup buffer_freer = [buffer] { free(buffer); };
+  const absl::Cleanup buffer_freer = [&buffer] { free(buffer); };
   ssize_t read_count;
   // NOLINTNEXTLINE(misc-include-cleaner)
   while ((read_count = getline(&buffer, &buf_size, input_stream)) > 0) {
@@ -316,11 +316,10 @@ static std::string StatusToErrorMessage(std::string_view message,
 }
 
 static absl::StatusOr<std::string> GetOptFlagOrFromEnv(
-  const absl::Flag<std::optional<std::string>> &flag,
-  std::string_view env_var) {
+  const absl::Flag<std::optional<std::string>> &flag, const char *env_var) {
   const std::optional<std::string> flag_value = absl::GetFlag(flag);
   if (!flag_value.has_value()) {
-    const char *value = getenv(std::string(env_var).c_str());
+    const char *value = getenv(env_var);
     if (value == nullptr) {
       return absl::InvalidArgumentError(
         absl::StrFormat("flag \"%s\" not provided either via commandline or "
diff --git a/fpga/database-parsers.cc b/fpga/database-parsers.cc
index c60e65e..ba3dc67 100644
--- a/fpga/database-parsers.cc
+++ b/fpga/database-parsers.cc
@@ -111,9 +111,9 @@ absl::StatusOr<T> Unmarshal(const rapidjson::Value &json) {
 
 template <typename T>
 inline absl::StatusOr<T> GetMember(const rapidjson::Value &json,
-                                   std::string_view name) {
+                                   const char *name) {
   OK_OR_RETURN(IsObject(json));
-  auto itr = json.FindMember(name.data());
+  auto itr = json.FindMember(name);
   if (itr == json.MemberEnd()) {
     return absl::InvalidArgumentError(absl::StrFormat(
       "json attribute \"%s\" not found in %s", name, ValueAsString(json)));
@@ -123,9 +123,9 @@ inline absl::StatusOr<T> GetMember(const rapidjson::Value &json,
 
 template <typename T>
 inline absl::StatusOr<std::optional<T>> OptGetMember(
-  const rapidjson::Value &json, std::string_view name) {
+  const rapidjson::Value &json, const char *name) {
   OK_OR_RETURN(IsObject(json));
-  auto itr = json.FindMember(name.data());
+  auto itr = json.FindMember(name);
   if (itr == json.MemberEnd()) {
     return std::optional<T>{};
   }
diff --git a/fpga/memory-mapped-file.cc b/fpga/memory-mapped-file.cc
index a73659c..1152bd8 100644
--- a/fpga/memory-mapped-file.cc
+++ b/fpga/memory-mapped-file.cc
@@ -33,9 +33,8 @@ class MemoryMappedFile final : public MemoryBlock {
 
 absl::StatusOr<std::unique_ptr<MemoryBlock>> MemoryMapFile(
   std::string_view path) {
-  const int fd = open(path.data(), O_RDONLY);
+  const int fd = open(std::string(path).c_str(), O_RDONLY);
   if (fd < 0) {
-    close(fd);
     return absl::Status(absl::ErrnoToStatus(
       errno, absl::StrFormat("could not open file: %s", path)));
   }