From 74176f5d16be153ce988f02de217b5adfa2126d5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Wed, 3 Apr 2024 12:09:46 -0700 Subject: [PATCH] [rbac] Fix read of uninitialized variable (#36244) When parsing `action` fails we don't touch that memory in the object loader, yet we still call `PostLoad` to fill in any other errors. In that case we are currently relying on undefined behavior to have this test work -- why msan didn't flag it upsets me. Default `action` to some safe value to avoid the undefined behavior, and log the bad action in the error message to ease debugging here in the future. Closes #36244 COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36244 from ctiller:rbac-undef d94b04a5086ab33d63ce0cbcd1687534583a36e3 PiperOrigin-RevId: 621599880 --- src/core/ext/filters/rbac/rbac_service_config_parser.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ext/filters/rbac/rbac_service_config_parser.cc b/src/core/ext/filters/rbac/rbac_service_config_parser.cc index f7264b3fe0a..bb9b529e9f7 100644 --- a/src/core/ext/filters/rbac/rbac_service_config_parser.cc +++ b/src/core/ext/filters/rbac/rbac_service_config_parser.cc @@ -203,7 +203,7 @@ struct RbacConfig { ValidationErrors* errors); }; - int action; + int action = static_cast(Rbac::Action::kDeny); std::map policies; // Defaults to kNone since its json field is optional. Rbac::AuditCondition audit_condition = Rbac::AuditCondition::kNone; @@ -801,7 +801,7 @@ void RbacConfig::RbacPolicy::Rules::JsonPostLoad(const Json& json, if (rbac_action != Rbac::Action::kAllow && rbac_action != Rbac::Action::kDeny) { ValidationErrors::ScopedField field(errors, ".action"); - errors->AddError("unknown action"); + errors->AddError(absl::StrCat("unknown action ", rbac_action)); } // Parse and validate audit_condition field. auto condition = LoadJsonObjectField(json.object(), args,