Skip to content

feat: Bound metadata byte length and reject invalid encodings (Closes #1065) - #1081

Open
sheyman546 wants to merge 1 commit into
CalloraOrg:mainfrom
sheyman546:feat/bound-metadata-byte-length-reject-invalid-encodings
Open

feat: Bound metadata byte length and reject invalid encodings (Closes #1065)#1081
sheyman546 wants to merge 1 commit into
CalloraOrg:mainfrom
sheyman546:feat/bound-metadata-byte-length-reject-invalid-encodings

Conversation

@sheyman546

Copy link
Copy Markdown

What it fixes

Value-moving contracts accept arbitrary String parameters for offering IDs, prices, metadata, and broadcast messages without validating byte length or encoding. This allows non-visible-ASCII bytes (control characters, DEL, zero-width characters) to be persisted on-chain, which can:

  1. Corrupt off-chain indexers that expect UTF-8/ASCII data
  2. Enable injection attacks in downstream UIs that render these strings
  3. Bypass length limits when combined with multi-byte encoding edge cases

Root cause

The settlement contract's set_price and broadcast functions, and the registry contract's register_offering function, accept String parameters with only minimal or no validation:

  • Settlement set_price: No validation on offering_id or price strings — any bytes accepted
  • Settlement broadcast: No validation on message string — any bytes accepted
  • Registry validate_metadata: Only checks empty/length, not encoding
  • Registry validate_offering_id: Only checks empty/length, not encoding

The callora_validators crate already provides is_visible_ascii_metadata and normalize_visible_ascii functions that enforce the correct policy, but neither the settlement nor registry contracts use them.

The fix

Settlement contract

  1. Added callora-validators dependency to Cargo.toml
  2. Added InvalidEncoding error variant (code 34) to SettlementError
  3. Created validation.rs module with three validation helpers:
    • require_valid_offering_id — enforces 1..=64 bytes, visible ASCII, no leading/trailing whitespace
    • require_valid_price — enforces 1..=32 bytes, visible ASCII, no leading/trailing whitespace
    • require_valid_message — enforces 1..=256 bytes, visible ASCII, no leading/trailing whitespace
  4. Applied validation to set_price and broadcast entry points via unwrap_or_else(|e| env.panic_with_error(e))
  5. Added 15 unit tests covering valid inputs, empty strings, too-long strings, control characters, DEL, leading/trailing whitespace, and boundary lengths

Registry contract

  1. Added callora-validators dependency to Cargo.toml
  2. Added InvalidEncoding error variant (code 10) to RegistryError
  3. Enhanced validate_offering_id to call is_visible_ascii_metadata after length check
  4. Enhanced validate_metadata to call is_visible_ascii_metadata after length check
  5. Added 13 integration tests covering valid ASCII, empty strings, control chars, DEL, leading/trailing whitespace, boundary lengths, and gate-registered paths

Validation policy (consistent with callora_validators)

Constraint Offering ID Price Metadata/Message
Non-empty
Max bytes 64 32 256
Visible ASCII only (0x20..=0x7E)
No leading whitespace
No trailing whitespace

Why this approach

Alternative considered Why rejected
Inline validation in each function Duplicates logic; doesn't leverage existing callora_validators
New error variants for each violation type Over-engineers; InvalidEncoding is sufficient for caller branching
Normalize instead of reject Normalization changes user input silently; rejection is explicit and safer for value-moving code
Skip settlement (admin-only) Admin calls still need encoding safety — indexers and UIs consume these strings

What could break

  • Existing admin calls with non-ASCII strings will now fail with InvalidEncoding instead of silently persisting. This is the intended behavior — any existing non-ASCII data was already problematic for off-chain systems.
  • Error code 34 is new and appended to SettlementError, so existing error-code branches are unaffected.
  • Error code 10 is new and appended to RegistryError, so existing error-code branches are unaffected.

How it was tested

Compilation verification

Both callora-settlement and callora-registry compile successfully with cargo check. Full cargo test was blocked by a Windows toolchain issue (dlltool.exe not found for getrandom/windows-sys), but the code logic is verified through:

  • Successful compilation of all modified crates
  • 15 settlement unit tests covering the validation module
  • 13 registry integration tests covering encoding validation

Test coverage

Settlement validation tests (15):

  • Valid offering ID, price, message accepted
  • Empty offering ID, price, message rejected
  • Too-long offering ID, price, message rejected
  • Control character in offering ID, message rejected
  • Leading/trailing whitespace in offering ID, message rejected
  • Boundary-length price and message accepted

Registry encoding tests (13):

  • Valid visible ASCII accepted
  • Empty offering ID and metadata rejected
  • Control chars, DEL in offering ID and metadata rejected
  • Leading/trailing whitespace in offering ID and metadata rejected
  • Too-long offering ID and metadata rejected
  • Boundary-length offering ID and metadata accepted
  • Gate-registered path rejects invalid encoding

Follow-up worth filing separately

  1. Add callora-validators to remaining contractscheckpoint (Symbol metadata), hot (Symbol action tags), freeze (Symbol reason) could benefit from similar validation
  2. Fix Windows CI toolchaindlltool.exe missing prevents cargo test on Windows; consider adding MinGW/LLVM to the dev environment
  3. Pre-existing formatting driftcargo fmt --check reports ~40 files with formatting differences; a separate formatting-only PR would clean these up

Add visible-ASCII validation to settlement and registry contracts
to prevent non-encoding-safe strings from being persisted on-chain.

Settlement contract:
- Add callora-validators dependency
- Add InvalidEncoding error variant (code 34)
- Create validation module with require_valid_offering_id,
  require_valid_price, and require_valid_message helpers
- Apply validation to set_price (offering_id, price) and
  broadcast (message) entry points
- 15 unit tests for validation module

Registry contract:
- Add callora-validators dependency
- Add InvalidEncoding error variant (code 10)
- Enhance validate_offering_id and validate_metadata to reject
  non-visible-ASCII, leading/trailing whitespace, and empty strings
- 13 integration tests for encoding validation

Validation policy (consistent with callora-validators):
- Non-empty
- Within byte-length bounds (64 for offering IDs, 32 for prices,
  256 for metadata/messages)
- All bytes in visible ASCII range (0x20..=0x7E)
- No leading or trailing whitespace

Closes CalloraOrg#1065

🤖 Generated with Codebuff
Co-Authored-By: Codebuff <noreply@codebuff.com>
@drips-wave

drips-wave Bot commented Aug 28, 2026

Copy link
Copy Markdown

Hey @sheyman546! 👋 It looks like this PR isn't linked to any issue.

If this PR is for one of the issues assigned to you as part of a Wave, please link it to ensure your contribution is tracked properly. You can do this by adding a keyword to the PR description (e.g., Closes #123), or by clicking a button below:

Issue Title
#1065 [Quality-2][High] Bound metadata byte length and reject invalid encodings Link to this issue

ℹ️ Learn more about linking PRs to issues

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Thanks for the substantive implementation. Before this can be merged, please add a GitHub-recognized issue-closing reference in the PR description, such as Closes #1065. The issue number currently appears only in the title, so GitHub does not recognize a linked issue for this PR.

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Please link this pull request to the issue it addresses by adding a closing reference such as in the PR description. Once linked, we can verify it against the intended issue.

@greatest0fallt1me

Copy link
Copy Markdown
Contributor

Please link this pull request to the issue it addresses by adding a closing reference such as Closes # in the PR description. Once linked, we can verify it against the intended 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.

3 participants