Skip to content

Comments

fix(ibc-hooks): safely construct JSON message using Go struct and json.Marshal#278

Open
odaysec wants to merge 1 commit intocosmos:mainfrom
odaysec:patch-1
Open

fix(ibc-hooks): safely construct JSON message using Go struct and json.Marshal#278
odaysec wants to merge 1 commit intocosmos:mainfrom
odaysec:patch-1

Conversation

@odaysec
Copy link

@odaysec odaysec commented Nov 13, 2025

This pull request addresses a potential security on issue #277 and reliability issue related to manual JSON string construction with interpolated, user-controlled values. Previously, the OnAcknowledgementPacketOverride method built the sudoMsg JSON payload using fmt.Sprintf, directly interpolating the ackAsJson value into a JSON string. This approach risks incorrect JSON formatting and potential injection vulnerabilities.

Changes

  • Replaced manual JSON string construction with a structured approach.
  • Introduced a Go struct representing the message payload.
  • Used json.Marshal to safely encode the struct into JSON, ensuring proper escaping and quoting of all fields.

This change improves code safety, maintainability, and ensures compliance with Go’s best practices for JSON handling.

Affected Area

  • sudoMsg := []byte(fmt.Sprintf(
    `{"ibc_lifecycle_complete": {"ibc_ack": {"channel": "%s", "sequence": %d, "ack": %s, "success": %s}}}`,
    packet.SourceChannel, packet.Sequence, ackAsJson, success))
    specifically the OnAcknowledgementPacketOverride method.

@github-actions github-actions bot added the ibc-hooks Label for items related to Osmosis' ibc-hooks implementation label Nov 13, 2025
@odaysec
Copy link
Author

odaysec commented Nov 13, 2025

/cc @gjermundgaraba. I kindly request that you urgently consider merging this pull request at your earliest convenience.

odaysec added a commit to odaysec/ibc-apps that referenced this pull request Nov 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ibc-hooks Label for items related to Osmosis' ibc-hooks implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant