Let callers disable --flagfile, --fromenv, and --tryfromenv in Abseil This is primarily intended for privileged (setuid) processes. Accessing the system prior to dropping privileges can cause command-lines coming from untrusted sources to leak data from the environment. PiperOrigin-RevId: 959814215 Change-Id: I371d796f88e3036a6bf579ea11768714baedc091
diff --git a/absl/flags/internal/parse.h b/absl/flags/internal/parse.h index 10c531b..aa29dea 100644 --- a/absl/flags/internal/parse.h +++ b/absl/flags/internal/parse.h
@@ -63,6 +63,12 @@ // misspellings. std::vector<std::string> GetMisspellingHints(absl::string_view flag); +// IsIndirectFlagExpansionEnabled() +// +// If DisableFlagfileAndEnvParsing() has been called, returns false. Otherwise, +// returns true. +[[nodiscard]] bool IsIndirectFlagExpansionEnabled(); + } // namespace flags_internal ABSL_NAMESPACE_END } // namespace absl
diff --git a/absl/flags/parse.cc b/absl/flags/parse.cc index 4961930..736dc8b 100644 --- a/absl/flags/parse.cc +++ b/absl/flags/parse.cc
@@ -18,6 +18,7 @@ #include <stdlib.h> #include <algorithm> +#include <atomic> #include <cstdint> #include <cstdlib> #include <fstream> @@ -64,6 +65,8 @@ namespace flags_internal { namespace { +ABSL_CONST_INIT std::atomic<bool> g_disable_indirect_flag_expansion(false); + absl::Mutex& ProcessingChecksMutex() { static absl::NoDestructor<absl::Mutex> mutex; return *mutex; @@ -354,6 +357,12 @@ // etc. bool ReadFlagfiles(const std::vector<std::string>& flagfiles, std::vector<ArgsList>& input_args) { + if (!flags_internal::IsIndirectFlagExpansionEnabled()) { + flags_internal::ReportUsageError( + "Skipping expansion of --flagfile as it is disabled.", false); + return true; + } + bool success = true; for (auto it = flagfiles.rbegin(); it != flagfiles.rend(); ++it) { ArgsList al; @@ -376,6 +385,13 @@ bool ReadFlagsFromEnv(const std::vector<std::string>& flag_names, std::vector<ArgsList>& input_args, bool fail_on_absent_in_env) { + if (!flags_internal::IsIndirectFlagExpansionEnabled()) { + flags_internal::ReportUsageError( + "Skipping expansion of --fromenv/--tryfromenv as it is disabled.", + false); + return true; + } + bool success = true; std::vector<std::string> args; @@ -921,8 +937,16 @@ : HelpMode::kNone; } +bool IsIndirectFlagExpansionEnabled() { + return !g_disable_indirect_flag_expansion; +} + } // namespace flags_internal +void DisableFlagfileAndEnvParsing() { + flags_internal::g_disable_indirect_flag_expansion = true; +} + void ParseAbseilFlagsOnly(int argc, char* argv[], std::vector<char*>& positional_args, std::vector<UnrecognizedFlag>& unrecognized_flags) {
diff --git a/absl/flags/parse.h b/absl/flags/parse.h index f2a5cb1..fdb3d1e 100644 --- a/absl/flags/parse.h +++ b/absl/flags/parse.h
@@ -124,6 +124,21 @@ // `ParseAbseilFlagsOnly`. std::vector<char*> ParseCommandLine(int argc, char* argv[]); +// DisableFlagfileAndEnvParsing() +// +// Disables the processing of flags that load values from secondary sources, +// specifically `--flagfile`, `--fromenv`, and `--tryfromenv`. When disabled, +// occurrences of these flags on the command line are skipped without opening +// files or inspecting environment variables, and a warning is printed to +// stderr. Direct command-line flags passed via argv are still parsed normally. +// +// This is primarily intended as a security precaution for privileged or setuid +// processes parsing untrusted command lines prior to dropping privileges. +// +// Should only be called in `main()` before calling `absl::ParseCommandLine()` +// or `absl::ParseAbseilFlagsOnly()`. +void DisableFlagfileAndEnvParsing(); + ABSL_NAMESPACE_END } // namespace absl
diff --git a/absl/flags/parse_test.cc b/absl/flags/parse_test.cc index 08eb81a..138cf6e 100644 --- a/absl/flags/parse_test.cc +++ b/absl/flags/parse_test.cc
@@ -1091,4 +1091,65 @@ // -------------------------------------------------------------------- +TEST_F(ParseDeathTest, DisableFlagfileAndEnvParsing) { + EXPECT_EXIT( + ([]() { + absl::DisableFlagfileAndEnvParsing(); + std::string flagfile_flag; + const char* args[] = { + "testbin", + "--string_flag=abc", + GetFlagfileFlag({{"parse_test.ff_disabled", {"--int_flag=123"}}}, + flagfile_flag), + }; + InvokeParse(args); + exit(absl::GetFlag(FLAGS_int_flag) == 123 || + absl::GetFlag(FLAGS_string_flag) != "abc" + ? 1 + : 0); + })(), + testing::ExitedWithCode(0), + "Skipping expansion of --flagfile as it is disabled"); + + EXPECT_EXIT(([]() { + absl::DisableFlagfileAndEnvParsing(); + ScopedSetEnv set_int_flag("FLAGS_int_flag", "123"); + const char* args[] = {"testbin", "--string_flag=abc", + "--fromenv=int_flag"}; + InvokeParse(args); + exit(absl::GetFlag(FLAGS_int_flag) == 123 || + absl::GetFlag(FLAGS_string_flag) != "abc" + ? 1 + : 0); + })(), + testing::ExitedWithCode(0), + "Skipping expansion of --fromenv/--tryfromenv as it is disabled"); + + EXPECT_EXIT(([]() { + absl::DisableFlagfileAndEnvParsing(); + ScopedSetEnv set_int_flag("FLAGS_int_flag", "123"); + const char* args[] = {"testbin", "--string_flag=abc", + "--tryfromenv=int_flag"}; + InvokeParse(args); + exit(absl::GetFlag(FLAGS_int_flag) == 123 || + absl::GetFlag(FLAGS_string_flag) != "abc" + ? 1 + : 0); + })(), + testing::ExitedWithCode(0), + "Skipping expansion of --fromenv/--tryfromenv as it is disabled"); +} + +TEST_F(ParseDeathTest, IsIndirectFlagExpansionEnabledWorks) { + EXPECT_EXIT( + ([]() { + if (!absl::flags_internal::IsIndirectFlagExpansionEnabled()) { + exit(1); + } + absl::DisableFlagfileAndEnvParsing(); + exit(absl::flags_internal::IsIndirectFlagExpansionEnabled() ? 2 : 0); + })(), + testing::ExitedWithCode(0), ""); +} + } // namespace