From f790d4dfe03f506dc481fe22725890ecd746a2f1 Mon Sep 17 00:00:00 2001 From: elle <0xllx0@noreply.codeberg.org> Date: Wed, 3 Dec 2025 16:53:19 +0100 Subject: [PATCH] bug: signature: modify URL after error check (#10204) Modifies the parsed URL after the error check. Avoids a potential `nil`-pointer de-reference if an invalid URL string argument is provided. No current code path triggers the `nil`-pointer de-reference. Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/10204 Reviewed-by: Gusted Co-authored-by: elle <0xllx0@noreply.codeberg.org> Co-committed-by: elle <0xllx0@noreply.codeberg.org> --- services/federation/signature_service.go | 3 ++- tests/integration/api_federation_httpsig_test.go | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/services/federation/signature_service.go b/services/federation/signature_service.go index fd8cbb39cd..8912db789a 100644 --- a/services/federation/signature_service.go +++ b/services/federation/signature_service.go @@ -23,11 +23,12 @@ import ( // Factory function for ActorID. Created struct is asserted to be valid func NewActorIDFromKeyID(ctx context.Context, uri string) (fm.ActorID, error) { parsedURI, err := url.Parse(uri) - parsedURI.Fragment = "" if err != nil { return fm.ActorID{}, err } + parsedURI.Fragment = "" + actionsUser := user.NewAPServerActor() clientFactory, err := activitypub.GetClientFactory(ctx) if err != nil { diff --git a/tests/integration/api_federation_httpsig_test.go b/tests/integration/api_federation_httpsig_test.go index a194452dd2..5003f691d4 100644 --- a/tests/integration/api_federation_httpsig_test.go +++ b/tests/integration/api_federation_httpsig_test.go @@ -19,6 +19,7 @@ import ( "forgejo.org/modules/test" "forgejo.org/routers" "forgejo.org/services/contexttest" + "forgejo.org/services/federation" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -73,6 +74,14 @@ func TestFederationHttpSigValidation(t *testing.T) { assert.True(t, user.PublicKey.Valid) }) + t.Run("ValidateActorFromKeyID", func(t *testing.T) { + _, err := federation.NewActorIDFromKeyID(ctx, actorKeyID) + require.NoError(t, err) + + _, err = federation.NewActorIDFromKeyID(ctx, "http://bad.url/%^&") + require.Error(t, err) + }) + // Disable signature validation defer test.MockVariableValue(&setting.Federation.SignatureEnforced, false)()