RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-27 12:07:45 +00:00
Code Issues Activity Actions

Tests: Un-flake the recent TestEnvironment addition

Browse source 
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.
This commit is contained in:
Jelle Raaijmakers 2023-06-06 11:54:20 +02:00 • committed by Andreas Kling
parent 2cd4f135f0
commit 162a2b66eb
2 changed files with 13 additions and 29 deletions
Download patch file Download diff file Expand all files Collapse all files

40
Tests/LibC/TestEnvironment.cpp
View file

@ -5,40 +5,25 @@
*/ */
#include <LibTest/TestCase.h> #include <LibTest/TestCase.h>
#include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <string.h> #include <string.h>
static int putenv_from_stack(char const* environment_variable) TEST_CASE(putenv_overwrite_invalid_value)
{ {
char environment_buffer[32]; // Write an environment variable using the heap
auto result = snprintf(environment_buffer, 31, "%s", environment_variable); auto* heap_environment_value = new char[12];
VERIFY(result > 0); VERIFY(snprintf(heap_environment_value, 12, "TESTVAR=123") == 11);
return putenv(environment_buffer); auto result = putenv(heap_environment_value);
}
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");
EXPECT_EQ(result, 0); EXPECT_EQ(result, 0);
// Try to retrieve the variable after overwriting the stack // Try to retrieve the variable after overwriting the heap value
auto environment_variable = getenv_with_overwritten_stack("TESTVAR"); memset(heap_environment_value, 0, 12);
auto* environment_variable = getenv("TESTVAR");
EXPECT_EQ(environment_variable, nullptr); EXPECT_EQ(environment_variable, nullptr);
// Try to overwrite the variable now that it's zeroed out // Try to overwrite the variable now that it's zeroed out
char new_environment_value[32]; auto* new_environment_value = new char[12];
result = snprintf(new_environment_value, 31, "%s", "TESTVAR=456"); VERIFY(snprintf(new_environment_value, 12, "TESTVAR=456") == 11);
VERIFY(result > 0);
result = putenv(new_environment_value); result = putenv(new_environment_value);
EXPECT_EQ(result, 0); EXPECT_EQ(result, 0);
@ -48,9 +33,8 @@ TEST_CASE(putenv_overwrite_invalid_stack_value)
EXPECT_EQ(strcmp(environment_variable, "456"), 0); EXPECT_EQ(strcmp(environment_variable, "456"), 0);
// Overwrite and retrieve it again to test correct search behavior for '=' // Overwrite and retrieve it again to test correct search behavior for '='
char final_environment_value[32]; auto* final_environment_value = new char[12];
result = snprintf(final_environment_value, 31, "%s", "TESTVAR=789"); VERIFY(snprintf(final_environment_value, 12, "TESTVAR=789") == 11);
VERIFY(result > 0);
result = putenv(final_environment_value); result = putenv(final_environment_value);
EXPECT_EQ(result, 0); EXPECT_EQ(result, 0);
environment_variable = getenv("TESTVAR"); environment_variable = getenv("TESTVAR");

2
Userland/Libraries/LibC/stdlib.cpp
View file

@ -502,7 +502,7 @@ int putenv(char* new_var)
auto old_var_name_max_length = strnlen(old_var, new_var_name_len); 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)); char* old_eq = static_cast<char*>(memchr(old_var, '=', old_var_name_max_length + 1));
if (!old_eq) 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; auto old_var_name_len = old_eq - old_var;
if (new_var_name_len != old_var_name_len) if (new_var_name_len != old_var_name_len)