From 162a2b66eb1576d06fff5faf13d7a83f05311f09 Mon Sep 17 00:00:00 2001
From: Jelle Raaijmakers <jelle@gmta.nl>
Date: Tue, 6 Jun 2023 11:54:20 +0200
Subject: [PATCH] Tests: Un-flake the recent `TestEnvironment` addition

Depending on stack values being correctly and deterministically
overwritten was a bit too optimistic, to be honest. This new logic uses
a value on the heap.
---
 Tests/LibC/TestEnvironment.cpp     | 40 +++++++++---------------------
 Userland/Libraries/LibC/stdlib.cpp |  2 +-
 2 files changed, 13 insertions(+), 29 deletions(-)

diff --git a/Tests/LibC/TestEnvironment.cpp b/Tests/LibC/TestEnvironment.cpp
index adb1f632faa..08795d96483 100644
--- a/Tests/LibC/TestEnvironment.cpp
+++ b/Tests/LibC/TestEnvironment.cpp
@@ -5,40 +5,25 @@
  */
 
 #include <LibTest/TestCase.h>
-#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 
-static int putenv_from_stack(char const* environment_variable)
+TEST_CASE(putenv_overwrite_invalid_value)
 {
-    char environment_buffer[32];
-    auto result = snprintf(environment_buffer, 31, "%s", environment_variable);
-    VERIFY(result > 0);
-    return putenv(environment_buffer);
-}
-
-static char const* getenv_with_overwritten_stack(char const* environment_variable_name)
-{
-    char environment_buffer[32];
-    memset(environment_buffer, ' ', 31);
-    environment_buffer[31] = 0;
-    return getenv(environment_variable_name);
-}
-
-TEST_CASE(putenv_overwrite_invalid_stack_value)
-{
-    // Write an environment variable using a stack value
-    auto result = putenv_from_stack("TESTVAR=123");
+    // Write an environment variable using the heap
+    auto* heap_environment_value = new char[12];
+    VERIFY(snprintf(heap_environment_value, 12, "TESTVAR=123") == 11);
+    auto result = putenv(heap_environment_value);
     EXPECT_EQ(result, 0);
 
-    // Try to retrieve the variable after overwriting the stack
-    auto environment_variable = getenv_with_overwritten_stack("TESTVAR");
+    // Try to retrieve the variable after overwriting the heap value
+    memset(heap_environment_value, 0, 12);
+    auto* environment_variable = getenv("TESTVAR");
     EXPECT_EQ(environment_variable, nullptr);
 
     // Try to overwrite the variable now that it's zeroed out
-    char new_environment_value[32];
-    result = snprintf(new_environment_value, 31, "%s", "TESTVAR=456");
-    VERIFY(result > 0);
+    auto* new_environment_value = new char[12];
+    VERIFY(snprintf(new_environment_value, 12, "TESTVAR=456") == 11);
     result = putenv(new_environment_value);
     EXPECT_EQ(result, 0);
 
@@ -48,9 +33,8 @@ TEST_CASE(putenv_overwrite_invalid_stack_value)
     EXPECT_EQ(strcmp(environment_variable, "456"), 0);
 
     // Overwrite and retrieve it again to test correct search behavior for '='
-    char final_environment_value[32];
-    result = snprintf(final_environment_value, 31, "%s", "TESTVAR=789");
-    VERIFY(result > 0);
+    auto* final_environment_value = new char[12];
+    VERIFY(snprintf(final_environment_value, 12, "TESTVAR=789") == 11);
     result = putenv(final_environment_value);
     EXPECT_EQ(result, 0);
     environment_variable = getenv("TESTVAR");
diff --git a/Userland/Libraries/LibC/stdlib.cpp b/Userland/Libraries/LibC/stdlib.cpp
index cae596a0ab1..bdda8c4310c 100644
--- a/Userland/Libraries/LibC/stdlib.cpp
+++ b/Userland/Libraries/LibC/stdlib.cpp
@@ -502,7 +502,7 @@ int putenv(char* new_var)
         auto old_var_name_max_length = strnlen(old_var, new_var_name_len);
         char* old_eq = static_cast<char*>(memchr(old_var, '=', old_var_name_max_length + 1));
         if (!old_eq)
-            continue; // possibly freed or overwritten value
+            continue; // name is longer, or possibly freed or overwritten value
 
         auto old_var_name_len = old_eq - old_var;
         if (new_var_name_len != old_var_name_len)