diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c10c4f..9110781e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Changed - Change no-op methods in `Shield::Hash` to `abstract def`s +### Fixed +- Limit lengths of OAuth client and bearer login names to mitigate potential DoS + ## [1.2.0] - 2024-07-05 ### Fixed diff --git a/docs/14-I18N.md b/docs/14-I18N.md index b59288b4..fb2962ec 100644 --- a/docs/14-I18N.md +++ b/docs/14-I18N.md @@ -200,6 +200,7 @@ en: name_invalid: Name is not valid name_not_allowed: The provided name is not allowed name_required: Name is required + name_too_long: Name cannot be longer than %{max} characters password_length_invalid: Password must be between %{min} and %{max} characters long password_notify_required: Password notification was not set password_required: Password is required diff --git a/spec/shield/operations/mixins/validate_bearer_login_spec.cr b/spec/shield/operations/mixins/validate_bearer_login_spec.cr index 3848dec9..1fa32ecf 100644 --- a/spec/shield/operations/mixins/validate_bearer_login_spec.cr +++ b/spec/shield/operations/mixins/validate_bearer_login_spec.cr @@ -91,6 +91,22 @@ describe Shield::ValidateBearerLogin do end end + it "rejects long name" do + user = UserFactory.create + + SaveBearerLogin.create(params( + user_id: user.id, + active_at: Time.utc, + name: "t" * 300, + token_digest: "abc", + scopes: [BearerScope.new(Api::Posts::Index).to_s] + )) do |operation, bearer_login| + bearer_login.should be_nil + + operation.name.should have_error("operation.error.name_too_long") + end + end + it "requires a valid name format" do user = UserFactory.create diff --git a/spec/shield/operations/mixins/validate_oauth_client_spec.cr b/spec/shield/operations/mixins/validate_oauth_client_spec.cr index 037ca53c..dbb85360 100644 --- a/spec/shield/operations/mixins/validate_oauth_client_spec.cr +++ b/spec/shield/operations/mixins/validate_oauth_client_spec.cr @@ -100,6 +100,24 @@ describe Shield::ValidateOauthClient do end end + it "rejects long name" do + user = UserFactory.create + + SaveOauthClient.create( + params( + active_at: Time.utc, + name: "c" * 300, + secret_digest: "a1b2c3", + user_id: user.id + ), + redirect_uris: ["https://example.com/oauth/callback"], + ) do |operation, oauth_client| + oauth_client.should be_nil + + operation.name.should have_error("operation.error.name_too_long") + end + end + it "requires a valid name format" do user = UserFactory.create diff --git a/src/shield/operations/mixins/validate_bearer_login.cr b/src/shield/operations/mixins/validate_bearer_login.cr index 54510c9f..c81d7933 100644 --- a/src/shield/operations/mixins/validate_bearer_login.cr +++ b/src/shield/operations/mixins/validate_bearer_login.cr @@ -5,8 +5,10 @@ module Shield::ValidateBearerLogin before_save do ensure_scopes_unique - validate_name_required validate_user_id_required + + validate_name_required + validate_name_length validate_name_valid validate_name_unique @@ -27,6 +29,14 @@ module Shield::ValidateBearerLogin validate_required name, message: Rex.t(:"operation.error.name_required") end + private def validate_name_length + max = 255 + + validate_size_of name, + max: max, + message: Rex.t(:"operation.error.name_too_long", max: max) + end + private def validate_user_id_required validate_required user_id, message: Rex.t(:"operation.error.user_id_required") diff --git a/src/shield/operations/mixins/validate_oauth_client.cr b/src/shield/operations/mixins/validate_oauth_client.cr index 81f0bd36..6de1ec60 100644 --- a/src/shield/operations/mixins/validate_oauth_client.cr +++ b/src/shield/operations/mixins/validate_oauth_client.cr @@ -7,6 +7,7 @@ module Shield::ValidateOauthClient limit_redirect_uris_count validate_name_required + validate_name_length validate_name_unique validate_name_valid validate_name_allowed @@ -22,8 +23,15 @@ module Shield::ValidateOauthClient include Lucille::ValidateUserExists private def validate_name_required - validate_required name, - message: Rex.t(:"operation.error.name_required") + validate_required name, message: Rex.t(:"operation.error.name_required") + end + + private def validate_name_length + max = 255 + + validate_size_of name, + max: max, + message: Rex.t(:"operation.error.name_too_long", max: max) end private def validate_name_unique