Skip to content

Validate submit EVM address inputs#7

Open
TUPM96 wants to merge 2 commits into
BitgesellOfficial:mainfrom
TUPM96:codex/validate-evm-address-inputs
Open

Validate submit EVM address inputs#7
TUPM96 wants to merge 2 commits into
BitgesellOfficial:mainfrom
TUPM96:codex/validate-evm-address-inputs

Conversation

@TUPM96

@TUPM96 TUPM96 commented May 25, 2026

Copy link
Copy Markdown

Summary

  • validate the original EVM address input format before normalizing it
  • reuse that validation for both BGL->WBGL and WBGL->BGL submit handlers
  • add regression coverage for malformed, short, long, non-hex, checksummed, lowercase, and no-prefix address inputs

Why

The previous handlers called common.HexToAddress(input).Hex() before validation. That can turn malformed inputs such as 0x1 or non-address strings into padded/canonical addresses, allowing bad user input past the validation gate.

Validation

  • go test ./workers/handlers
  • go test ./...

Refs #3.
Submitting as a focused bridge robustness/test-coverage improvement under BitgesellOfficial/bitgesell#39 if maintainers consider it eligible. Payout details can be provided privately if approved.

Copilot AI review requested due to automatic review settings May 25, 2026 07:43

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens bridge submit handlers by validating EVM address inputs before they can be normalized into a canonical 20-byte address, preventing malformed user input from slipping past validation. It also centralizes the validation logic and adds regression tests for common malformed/edge-case address inputs.

Changes:

  • Introduces a shared validateEVMAddress helper and reuses it in both SubmitBGL and SubmitWBGL.
  • Updates submit handlers to validate the raw user-provided EVM address input rather than validating a normalized/padded version.
  • Adds unit tests covering malformed, short/long, non-hex, checksummed, lowercase, and no-prefix inputs.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
workers/handlers/SubmitWBGL.go Replaces inline EVM address validation with shared helper in the WBGL→BGL submit handler.
workers/handlers/SubmitBGL.go Replaces inline EVM address validation with shared helper in the BGL→WBGL submit handler.
workers/handlers/address.go Adds centralized EVM address validation helper.
workers/handlers/address_test.go Adds regression tests for the new EVM address validation helper.
Comments suppressed due to low confidence (2)

workers/handlers/SubmitWBGL.go:55

  • validateEVMAddress() accepts non-0x-prefixed addresses, but later SubmitWBGL compares req.EthAddress with address.Hex() (always 0x-prefixed) using strings.EqualFold. A valid no-prefix input will pass validation but fail the signature match check. Normalize req.EthAddress to a canonical 0x-prefixed form after validation (or require the prefix in validation) to keep behavior consistent.
	if err := validateEVMAddress(req.EthAddress); err != nil {
		log.Printf("Error validating Eth address '%s': %s\n", req.EthAddress, err.Error())
		responseJSON(w, &APIResponse{
			Status:  "error",
			Field:   "ethAddress",

workers/handlers/SubmitBGL.go:50

  • On EVM address validation failure, the response sets Field to "ethAddress", but the request payload field is "address" (BGLtoWBGLBindingRequest.Address). This makes the error response inconsistent for API clients and harder to map back to the submitted JSON field.
		responseJSON(w, &APIResponse{
			Status:  "error",
			Field:   "ethAddress",
			Message: "No ethereum address or invalid address provided",
		}, http.StatusBadRequest)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread workers/handlers/address.go Outdated
Comment on lines +11 to +15
if !common.IsHexAddress(address) {
return errors.New("invalid ethereum address format")
}

return ethav.Validate(common.HexToAddress(address).Hex())
@MyTH-zyxeon

Copy link
Copy Markdown

Maintainers - this looks like a useful bridge-hardening candidate in the Bitgesell improvement queue.

Why it seems worth reviewing:

  • The bug class is concrete: common.HexToAddress(input).Hex() can canonicalize malformed or short input before validation, so the old path risks accepting a value different from what the user submitted.
  • The fix is scoped to both submit handlers and a shared validateEVMAddress helper, with regression tests for malformed, short, long, non-hex, lowercase, checksummed, and bad-checksum inputs.
  • This is more safety-relevant than a pure docs cleanup because it protects bridge request validation before any downstream signing / submission path.

Suggested acceptance checks:

  1. Confirm the no-prefix policy end-to-end. validateEVMAddress() accepts de709f... without 0x, but SubmitWBGL later compares the submitted string to address.Hex() which is always 0x-prefixed. Either canonicalize the accepted input before that comparison or require the prefix at validation time, then add one handler-level regression for that policy.
  2. Confirm the SubmitBGL validation error Field should be address, not ethAddress, so API clients can map the error back to BGLtoWBGLBindingRequest.Address.
  3. Preserve the checksum boundary: lowercase addresses should pass, correctly checksummed mixed-case addresses should pass, and bad-checksum mixed-case addresses should fail without first normalizing them.
  4. Re-run go test ./workers/handlers and go test ./...; the helper-level tests are a good start, but one submit-handler test for the accepted/rejected input shape would make the API behavior explicit.
  5. For payout hygiene, confirm whether this should be routed under gobglbridge#3, bitgesell#39, or the broader Bitgesell improvement program before anyone claims payment.

No live bridge calls, wallet actions, secrets, dependency installs, or payment actions from me here; this is queue-triage help for a narrow PR that should be easy to approve or request changes on.

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