Skip to content

Commit

Permalink
refact: improved AsyncCallback.UnmarshalJSON validation
Browse files Browse the repository at this point in the history
Description
-----------
- Added overflow checking for uint64 ID conversion
- Added validation for decimal and negative values
- Added comprehensive test cases for edge cases
  • Loading branch information
0xObsidian committed Dec 20, 2024
1 parent 2856872 commit e556df8
Show file tree
Hide file tree
Showing 2 changed files with 75 additions and 4 deletions.
13 changes: 9 additions & 4 deletions x/ibc-hooks/move-hooks/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,21 +78,26 @@ func (a *AsyncCallback) UnmarshalJSON(bz []byte) error {
return fmt.Errorf("module_name cannot be empty")
}

// Validate module address format (assuming it should start with "0x")
// Validate module address format
if !strings.HasPrefix(ic.ModuleAddress, "0x") {
return fmt.Errorf("invalid module_address format: must start with '0x'")
}

// Handle ID based on type
// Handle ID based on type with overflow checking
switch v := ic.Id.(type) {
case float64:
if v < 0 || v > float64(^uint64(0)) || v != float64(uint64(v)) {
return fmt.Errorf("id value out of range or contains decimals")
}
a.Id = uint64(v)
case string:
var err error
var parsed float64
if err = json.Unmarshal([]byte(v), &parsed); err != nil {
if err := json.Unmarshal([]byte(v), &parsed); err != nil {
return fmt.Errorf("invalid id format: %w", err)
}
if parsed < 0 || parsed > float64(^uint64(0)) || parsed != float64(uint64(parsed)) {
return fmt.Errorf("id value out of range or contains decimals")
}
a.Id = uint64(parsed)
default:
return fmt.Errorf("invalid id type: expected string or number")
Expand Down
66 changes: 66 additions & 0 deletions x/ibc-hooks/move-hooks/message_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,4 +100,70 @@ func Test_Unmarshal_AsyncCallback(t *testing.T) {
require.Error(t, err)
require.Contains(t, err.Error(), "invalid character")
})

t.Run("id with decimal value", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": 99.5,
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})

t.Run("id with string decimal value", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": "99.5",
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})

t.Run("negative id value", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": -1,
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})

t.Run("negative string id value", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": "-1",
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})

t.Run("id value exceeding uint64 max", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": 18446744073709551616,
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})

t.Run("string id value exceeding uint64 max", func(t *testing.T) {
var callback movehooks.AsyncCallback
err := json.Unmarshal([]byte(`{
"id": "18446744073709551616",
"module_address": "0x1",
"module_name": "Counter"
}`), &callback)
require.Error(t, err)
require.Contains(t, err.Error(), "id value out of range or contains decimals")
})
}

0 comments on commit e556df8

Please sign in to comment.