From 64bb0dd8d918a95abb46535b81fb173dcc3c99e7 Mon Sep 17 00:00:00 2001 From: Matthew Pendrey Date: Wed, 14 Aug 2024 11:39:38 +0100 Subject: [PATCH 1/2] keys11 validate ids prior to using as seed for transmission schedule --- .changeset/selfish-foxes-call.md | 5 ++++ core/capabilities/remote/target/client.go | 9 ++++++-- core/capabilities/remote/trigger_publisher.go | 5 ++-- core/capabilities/remote/utils.go | 10 -------- .../capabilities/transmission/transmission.go | 11 +++++---- core/capabilities/validation/validation.go | 23 +++++++++++++++++++ 6 files changed, 45 insertions(+), 18 deletions(-) create mode 100644 .changeset/selfish-foxes-call.md create mode 100644 core/capabilities/validation/validation.go diff --git a/.changeset/selfish-foxes-call.md b/.changeset/selfish-foxes-call.md new file mode 100644 index 00000000000..7d001cac75f --- /dev/null +++ b/.changeset/selfish-foxes-call.md @@ -0,0 +1,5 @@ +--- +"chainlink": patch +--- + +#internal keys11 audit finding fix diff --git a/core/capabilities/remote/target/client.go b/core/capabilities/remote/target/client.go index 4273169d23e..8572efed155 100644 --- a/core/capabilities/remote/target/client.go +++ b/core/capabilities/remote/target/client.go @@ -12,6 +12,7 @@ import ( "github.com/smartcontractkit/chainlink/v2/core/capabilities/remote" "github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/target/request" "github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/types" + "github.com/smartcontractkit/chainlink/v2/core/capabilities/validation" "github.com/smartcontractkit/chainlink/v2/core/logger" ) @@ -172,8 +173,12 @@ func (c *client) Receive(ctx context.Context, msg *types.MessageBody) { } func GetMessageIDForRequest(req commoncap.CapabilityRequest) (string, error) { - if !remote.IsValidWorkflowOrExecutionID(req.Metadata.WorkflowID) || !remote.IsValidWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID) { - return "", errors.New("workflow ID and workflow execution ID in request metadata are invalid") + if err := validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil { + return "", fmt.Errorf("workflow ID is invalid: %w", err) + } + + if err := validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID); err != nil { + return "", fmt.Errorf("workflow execution ID is invalid: %w", err) } return req.Metadata.WorkflowID + req.Metadata.WorkflowExecutionID, nil diff --git a/core/capabilities/remote/trigger_publisher.go b/core/capabilities/remote/trigger_publisher.go index 23b778f6018..4aac821bc14 100644 --- a/core/capabilities/remote/trigger_publisher.go +++ b/core/capabilities/remote/trigger_publisher.go @@ -10,6 +10,7 @@ import ( "github.com/smartcontractkit/chainlink-common/pkg/capabilities/pb" "github.com/smartcontractkit/chainlink-common/pkg/services" "github.com/smartcontractkit/chainlink/v2/core/capabilities/remote/types" + "github.com/smartcontractkit/chainlink/v2/core/capabilities/validation" "github.com/smartcontractkit/chainlink/v2/core/logger" p2ptypes "github.com/smartcontractkit/chainlink/v2/core/services/p2p/types" ) @@ -102,8 +103,8 @@ func (p *triggerPublisher) Receive(_ context.Context, msg *types.MessageBody) { p.lggr.Errorw("sender not a member of its workflow DON", "capabilityId", p.capInfo.ID, "callerDonId", msg.CallerDonId, "sender", sender) return } - if !IsValidWorkflowOrExecutionID(req.Metadata.WorkflowID) { - p.lggr.Errorw("received trigger request with invalid workflow ID", "capabilityId", p.capInfo.ID, "workflowId", SanitizeLogString(req.Metadata.WorkflowID)) + if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil { + p.lggr.Errorw("received trigger request with invalid workflow ID", "capabilityId", p.capInfo.ID, "workflowId", SanitizeLogString(req.Metadata.WorkflowID), "err", err) return } p.lggr.Debugw("received trigger registration", "capabilityId", p.capInfo.ID, "workflowId", req.Metadata.WorkflowID, "sender", sender) diff --git a/core/capabilities/remote/utils.go b/core/capabilities/remote/utils.go index 7e303eefc8f..f9dc0567fea 100644 --- a/core/capabilities/remote/utils.go +++ b/core/capabilities/remote/utils.go @@ -19,7 +19,6 @@ import ( const ( maxLoggedStringLen = 256 - validWorkflowIDLen = 64 maxIDLen = 128 ) @@ -116,15 +115,6 @@ func SanitizeLogString(s string) string { return s + tooLongSuffix } -// Workflow IDs and Execution IDs are 32-byte hex-encoded strings -func IsValidWorkflowOrExecutionID(id string) bool { - if len(id) != validWorkflowIDLen { - return false - } - _, err := hex.DecodeString(id) - return err == nil -} - // Trigger event IDs and message IDs can only contain printable characters and must be non-empty func IsValidID(id string) bool { if len(id) == 0 || len(id) > maxIDLen { diff --git a/core/capabilities/transmission/transmission.go b/core/capabilities/transmission/transmission.go index b41be5bcaa5..9403f9e2696 100644 --- a/core/capabilities/transmission/transmission.go +++ b/core/capabilities/transmission/transmission.go @@ -4,8 +4,7 @@ import ( "fmt" "time" - "github.com/pkg/errors" - + "github.com/smartcontractkit/chainlink/v2/core/capabilities/validation" "github.com/smartcontractkit/libocr/permutation" "github.com/smartcontractkit/chainlink-common/pkg/capabilities" @@ -56,8 +55,12 @@ func GetPeerIDToTransmissionDelay(donPeerIDs []types.PeerID, req capabilities.Ca return nil, fmt.Errorf("failed to extract transmission config from request: %w", err) } - if req.Metadata.WorkflowID == "" || req.Metadata.WorkflowExecutionID == "" { - return nil, errors.New("workflow ID and workflow execution ID must be set in request metadata") + if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowID); err != nil { + return nil, fmt.Errorf("workflow ID is invalid: %w", err) + } + + if err = validation.ValidateWorkflowOrExecutionID(req.Metadata.WorkflowExecutionID); err != nil { + return nil, fmt.Errorf("workflow execution ID is invalid: %w", err) } transmissionID := req.Metadata.WorkflowID + req.Metadata.WorkflowExecutionID diff --git a/core/capabilities/validation/validation.go b/core/capabilities/validation/validation.go new file mode 100644 index 00000000000..00f24b8b71a --- /dev/null +++ b/core/capabilities/validation/validation.go @@ -0,0 +1,23 @@ +package validation + +import ( + "encoding/hex" + "errors" +) + +const ( + validWorkflowIDLen = 64 +) + +// Workflow IDs and Execution IDs are 32-byte hex-encoded strings +func ValidateWorkflowOrExecutionID(id string) error { + if len(id) != validWorkflowIDLen { + return errors.New("must be 32 bytes long") + } + _, err := hex.DecodeString(id) + if err != nil { + return errors.New("must be a hex-encoded string") + } + + return nil +} From 8df50d3be2a568ac3f589158057d9fd72289b609 Mon Sep 17 00:00:00 2001 From: Matthew Pendrey Date: Wed, 14 Aug 2024 13:17:02 +0100 Subject: [PATCH 2/2] changeset --- .changeset/selfish-foxes-call.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/selfish-foxes-call.md diff --git a/.changeset/selfish-foxes-call.md b/.changeset/selfish-foxes-call.md deleted file mode 100644 index 7d001cac75f..00000000000 --- a/.changeset/selfish-foxes-call.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -"chainlink": patch ---- - -#internal keys11 audit finding fix