Skip to content

fix(service/s3/validators): validate empty bucket to prevent panic in dnsCompatibleBucketName method#3377

Open
hanagantig wants to merge 1 commit intoaws:mainfrom
hanagantig:fix/s3-empty-bucket-panic
Open

fix(service/s3/validators): validate empty bucket to prevent panic in dnsCompatibleBucketName method#3377
hanagantig wants to merge 1 commit intoaws:mainfrom
hanagantig:fix/s3-empty-bucket-panic

Conversation

@hanagantig
Copy link
Copy Markdown

Description

Fixes panic when Bucket is empty in PutObject input validation.

The SDK currently allows an empty string bucket ("") to pass validation, which later leads to a panic in dnsCompatibleBucketName due to direct indexing (bucket[0]).

Before (bug):

  • Bucket == nil is validated
  • Bucket == "" passes validation
  • Panic occurs when accessing bucket[0]

After (fix):

  • Both Bucket == nil and Bucket == "" are treated as invalid
  • Proper validation error is returned instead of panic

Root Cause

In dnsCompatibleBucketName, the function assumes a non-empty string and directly accesses bucket[0]:

if !((bucket[0] > 96 && bucket[0] < 123) || (bucket[0] > 47 && bucket[0] < 58)) {

When bucket == "", this results in an out-of-bounds access and causes a runtime panic.

The validation layer (validateOpPutObjectInput) only checks for nil pointers, but does not validate empty string values, allowing invalid input to propagate.


Fix

Extend validation in validateOpPutObjectInput to reject empty bucket names:

if v.Bucket == nil || *v.Bucket == "" {
    invalidParams.Add(smithy.NewErrParamRequired("Bucket"))
}

Result

  • Prevents runtime panic caused by empty bucket names
  • Ensures consistent and safe input validation
  • Aligns behavior with expected SDK validation patterns (invalid input → error, not panic)

@hanagantig hanagantig requested a review from a team as a code owner April 10, 2026 19:56
@Madrigal
Copy link
Copy Markdown
Collaborator

Unfortunately, you are making code changes to a generated file. See the first line of that file

// Code generated by smithy-go-codegen DO NOT EDIT.

The service model is the source of truth for this, and it's owned by S3 directly, not by the SDK, so we can't accept this PR. You can create an issue and we can own reaching out to S3 about adding this validation on their model.

THAT SAID, it seems like the issue you are facing is with one of our customizations on dnsCompatibleBucketName. If instead of assuming a non-empty bucket we instead returned false on empty bucket (since empty names are not DNS compatible), would that solve your issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants