Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(node-api): hardened CustomValidator validation #2093

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

fix(node-api): hardened CustomValidator validation #2093

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9ced281
hardened node-api custom validator validation
abi87 Oct 22, 2024
b1e1edb
Merge branch 'main' into hardened-node-api-custom-validator
abi87 Oct 22, 2024
b053357
Merge branch 'main' into hardened-node-api-custom-validator
nidhi-singh02 Oct 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 7 additions & 22 deletions mod/node-api/engines/echo/vaildator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import (
"errors"
"fmt"
"net/http"
"regexp"
"strconv"

Comment on lines 26 to 28
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Consider updating the TODO comment to reflect recent changes

The recent updates to ValidateValidatorID and ValidateRoot begin to address the concerns mentioned in the TODO comment regarding strong typing and avoidance of repeated .Field().String() calls. Please update or remove the TODO comment if it's no longer applicable.

Would you like assistance in updating the TODO comment to reflect the remaining tasks?

"github.com/berachain/beacon-kit/mod/node-api/handlers/utils"
"github.com/berachain/beacon-kit/mod/primitives/pkg/common"
"github.com/berachain/beacon-kit/mod/primitives/pkg/crypto"
"github.com/go-playground/validator/v10"
"github.com/labstack/echo/v4"
)
Expand Down Expand Up @@ -130,11 +131,9 @@ func ValidateUint64(fl validator.FieldLevel) bool {
// validator identifier. It validates against a hex-encoded public key
// or a numeric validator index.
func ValidateValidatorID(fl validator.FieldLevel) bool {
valid, err := validateRegex(fl.Field().String(), `^0x[0-9a-fA-F]{1,96}$`)
if err != nil {
return false
}
if valid {
var key crypto.BLSPubkey
err := key.UnmarshalText([]byte(fl.Field().String()))
if err == nil {
return true
}
if ValidateUint64(fl) {
Expand All @@ -146,11 +145,8 @@ func ValidateValidatorID(fl validator.FieldLevel) bool {
// ValidateRoot checks if the provided field is a valid root.
// It validates against a 32 byte hex-encoded root with "0x" prefix.
func ValidateRoot(value string) bool {
valid, err := validateRegex(value, `^0x[0-9a-fA-F]{64}$`)
if err != nil {
return false
}
return valid
_, err := common.NewRootFromHex(value)
return err == nil
}

func ValidateValidatorStatus(fl validator.FieldLevel) bool {
Expand Down Expand Up @@ -179,17 +175,6 @@ func validateAllowedStrings(
return allowedValues[value]
}

func validateRegex(value string, hexPattern string) (bool, error) {
if value == "" {
return true, nil
}
matched, err := regexp.MatchString(hexPattern, value)
if err != nil {
return false, err
}
return matched, nil
}

func validateStateBlockIDs(value string, allowedValues map[string]bool) bool {
// Check if value is one of the allowed values
if validateAllowedStrings(value, allowedValues) {
Expand Down
Loading