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 <gusted@noreply.codeberg.org>
Co-authored-by: elle <0xllx0@noreply.codeberg.org>
Co-committed-by: elle <0xllx0@noreply.codeberg.org>
This commit is contained in:
elle 2025-12-03 16:53:19 +01:00 committed by Gusted
parent 655def254f
commit f790d4dfe0
2 changed files with 11 additions and 1 deletions

View file

@ -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 {

View file

@ -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)()