From 2bafd7daf542d985ee76d9079a30a602cb7be827 Mon Sep 17 00:00:00 2001 From: kim <89579420+NyaaaWhatsUpDoc@users.noreply.github.com> Date: Wed, 14 Feb 2024 11:13:38 +0000 Subject: [PATCH] [bugfix] add stricter checks during all stages of dereferencing remote AS objects (#2639) * add stricter checks during all stages of dereferencing remote AS objects * a comment --- cmd/gotosocial/action/testrig/testrig.go | 3 + internal/api/util/mime.go | 114 +++++++++++++++++- internal/api/util/mime_test.go | 75 ++++++++++++ internal/federation/dereferencing/account.go | 47 ++++---- .../federation/dereferencing/account_test.go | 23 ++++ .../dereferencing/dereferencer_test.go | 4 +- internal/federation/dereferencing/finger.go | 11 +- internal/federation/dereferencing/status.go | 25 +++- .../federation/dereferencing/status_test.go | 23 ++++ internal/federation/federatingactor.go | 56 +-------- internal/federation/federatingactor_test.go | 68 ----------- internal/transport/dereference.go | 7 ++ internal/transport/derefinstance.go | 33 ++++- internal/transport/finger.go | 14 +++ testrig/transportcontroller.go | 4 +- 15 files changed, 345 insertions(+), 162 deletions(-) create mode 100644 internal/api/util/mime_test.go diff --git a/cmd/gotosocial/action/testrig/testrig.go b/cmd/gotosocial/action/testrig/testrig.go index dc5f1c7dc..1220d6c23 100644 --- a/cmd/gotosocial/action/testrig/testrig.go +++ b/cmd/gotosocial/action/testrig/testrig.go @@ -107,6 +107,9 @@ return &http.Response{ StatusCode: 200, Body: r, + Header: http.Header{ + "Content-Type": req.Header.Values("Accept"), + }, }, nil }, "")) mediaManager := testrig.NewTestMediaManager(&state) diff --git a/internal/api/util/mime.go b/internal/api/util/mime.go index ad1b405cd..455a84de9 100644 --- a/internal/api/util/mime.go +++ b/internal/api/util/mime.go @@ -17,6 +17,8 @@ package util +import "strings" + const ( // Possible GoToSocial mimetypes. AppJSON = `application/json` @@ -24,7 +26,8 @@ AppXMLXRD = `application/xrd+xml` AppRSSXML = `application/rss+xml` AppActivityJSON = `application/activity+json` - AppActivityLDJSON = `application/ld+json; profile="https://www.w3.org/ns/activitystreams"` + appActivityLDJSON = `application/ld+json` // without profile + AppActivityLDJSON = appActivityLDJSON + `; profile="https://www.w3.org/ns/activitystreams"` AppJRDJSON = `application/jrd+json` // https://www.rfc-editor.org/rfc/rfc7033#section-10.2 AppForm = `application/x-www-form-urlencoded` MultipartForm = `multipart/form-data` @@ -32,3 +35,112 @@ TextHTML = `text/html` TextCSS = `text/css` ) + +// JSONContentType returns whether is application/json(;charset=utf-8)? content-type. +func JSONContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + return ok && len(p) == 1 && + p[0] == AppJSON +} + +// JSONJRDContentType returns whether is application/(jrd+)?json(;charset=utf-8)? content-type. +func JSONJRDContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + return ok && len(p) == 1 && + p[0] == AppJSON || + p[0] == AppJRDJSON +} + +// XMLContentType returns whether is application/xml(;charset=utf-8)? content-type. +func XMLContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + return ok && len(p) == 1 && + p[0] == AppXML +} + +// XMLXRDContentType returns whether is application/(xrd+)?xml(;charset=utf-8)? content-type. +func XMLXRDContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + return ok && len(p) == 1 && + p[0] == AppXML || + p[0] == AppXMLXRD +} + +// ASContentType returns whether is valid ActivityStreams content-types: +// - application/activity+json +// - application/ld+json;profile=https://w3.org/ns/activitystreams +func ASContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + if !ok { + return false + } + switch len(p) { + case 1: + return p[0] == AppActivityJSON + case 2: + return p[0] == appActivityLDJSON && + p[1] == "profile=https://www.w3.org/ns/activitystreams" || + p[1] == "profile=\"https://www.w3.org/ns/activitystreams\"" + default: + return false + } +} + +// NodeInfo2ContentType returns whether is nodeinfo schema 2.0 content-type. +func NodeInfo2ContentType(ct string) bool { + p := splitContentType(ct) + p, ok := isUTF8ContentType(p) + if !ok { + return false + } + switch len(p) { + case 1: + return p[0] == AppJSON + case 2: + return p[0] == AppJSON && + p[1] == "profile=\"http://nodeinfo.diaspora.software/ns/schema/2.0#\"" || + p[1] == "profile=http://nodeinfo.diaspora.software/ns/schema/2.0#" + default: + return false + } +} + +// isUTF8ContentType checks for a provided charset in given +// type parts list, removes it and returns whether is utf-8. +func isUTF8ContentType(p []string) ([]string, bool) { + const charset = "charset=" + const charsetUTF8 = charset + "utf-8" + for i, part := range p { + + // Only handle charset slice parts. + if part[:len(charset)] == charset { + + // Check if is UTF-8 charset. + ok := (part == charsetUTF8) + + // Drop this slice part. + _ = copy(p[i:], p[i+1:]) + p = p[:len(p)-1] + + return p, ok + } + } + return p, true +} + +// splitContentType splits content-type into semi-colon +// separated parts. useful when a charset is provided. +// note this also maps all chars to their lowercase form. +func splitContentType(ct string) []string { + s := strings.Split(ct, ";") + for i := range s { + s[i] = strings.TrimSpace(s[i]) + s[i] = strings.ToLower(s[i]) + } + return s +} diff --git a/internal/api/util/mime_test.go b/internal/api/util/mime_test.go new file mode 100644 index 000000000..6b12d1436 --- /dev/null +++ b/internal/api/util/mime_test.go @@ -0,0 +1,75 @@ +package util_test + +import ( + "testing" + + "github.com/superseriousbusiness/gotosocial/internal/api/util" +) + +func TestIsASContentType(t *testing.T) { + for _, test := range []struct { + Input string + Expect bool + }{ + { + Input: "application/activity+json", + Expect: true, + }, + { + Input: "application/activity+json; charset=utf-8", + Expect: true, + }, + { + Input: "application/activity+json;charset=utf-8", + Expect: true, + }, + { + Input: "application/activity+json ;charset=utf-8", + Expect: true, + }, + { + Input: "application/activity+json ; charset=utf-8", + Expect: true, + }, + { + Input: "application/ld+json;profile=https://www.w3.org/ns/activitystreams", + Expect: true, + }, + { + Input: "application/ld+json;profile=\"https://www.w3.org/ns/activitystreams\"", + Expect: true, + }, + { + Input: "application/ld+json ;profile=https://www.w3.org/ns/activitystreams", + Expect: true, + }, + { + Input: "application/ld+json ;profile=\"https://www.w3.org/ns/activitystreams\"", + Expect: true, + }, + { + Input: "application/ld+json ; profile=https://www.w3.org/ns/activitystreams", + Expect: true, + }, + { + Input: "application/ld+json ; profile=\"https://www.w3.org/ns/activitystreams\"", + Expect: true, + }, + { + Input: "application/ld+json; profile=https://www.w3.org/ns/activitystreams", + Expect: true, + }, + { + Input: "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"", + Expect: true, + }, + { + Input: "application/ld+json", + Expect: false, + }, + } { + if util.ASContentType(test.Input) != test.Expect { + t.Errorf("did not get expected result %v for input: %s", test.Expect, test.Input) + } + } +} diff --git a/internal/federation/dereferencing/account.go b/internal/federation/dereferencing/account.go index 7eec6b5b9..c3ad6be5e 100644 --- a/internal/federation/dereferencing/account.go +++ b/internal/federation/dereferencing/account.go @@ -606,6 +606,16 @@ func (d *Dereferencer) enrichAccount( } if account.Username == "" { + // Assume the host from the + // ActivityPub representation. + id := ap.GetJSONLDId(apubAcc) + if id == nil { + return nil, nil, gtserror.New("no id property found on person, or id was not an iri") + } + + // Get IRI host value. + accHost := id.Host + // No username was provided, so no webfinger was attempted earlier. // // Now we have a username we can attempt again, to ensure up-to-date @@ -616,42 +626,37 @@ func (d *Dereferencer) enrichAccount( // https://example.org/@someone@somewhere.else and we've been redirected // from example.org to somewhere.else: we want to take somewhere.else // as the accountDomain then, not the example.org we were redirected from. - - // Assume the host from the returned - // ActivityPub representation. - id := ap.GetJSONLDId(apubAcc) - if id == nil { - return nil, nil, gtserror.New("no id property found on person, or id was not an iri") - } - - // Get IRI host value. - accHost := id.Host - latestAcc.Domain, _, err = d.fingerRemoteAccount(ctx, tsport, latestAcc.Username, accHost, ) if err != nil { - // We still couldn't webfinger the account, so we're not certain - // what the accountDomain actually is. Still, we can make a solid - // guess that it's the Host of the ActivityPub URI of the account. - // If we're wrong, we can just try again in a couple days. - log.Errorf(ctx, "error webfingering[2] remote account %s@%s: %v", latestAcc.Username, accHost, err) - latestAcc.Domain = accHost + // Webfingering account still failed, so we're not certain + // what the accountDomain actually is. Exit here for safety. + return nil, nil, gtserror.Newf( + "error webfingering remote account %s@%s: %w", + latestAcc.Username, accHost, err, + ) } } if latestAcc.Domain == "" { // Ensure we have a domain set by this point, // otherwise it gets stored as a local user! - // - // TODO: there is probably a more granular way - // way of checking this in each of the above parts, - // and honestly it could do with a smol refactor. return nil, nil, gtserror.Newf("empty domain for %s", uri) } + // Ensure the final parsed account URI / URL matches + // the input URI we fetched (or received) it as. + if expect := uri.String(); latestAcc.URI != expect && + latestAcc.URL != expect { + return nil, nil, gtserror.Newf( + "dereferenced account uri %s does not match %s", + latestAcc.URI, expect, + ) + } + /* BY THIS POINT we have more or less a fullly-formed representation of the target account, derived from diff --git a/internal/federation/dereferencing/account_test.go b/internal/federation/dereferencing/account_test.go index ef1eddb91..f99012904 100644 --- a/internal/federation/dereferencing/account_test.go +++ b/internal/federation/dereferencing/account_test.go @@ -19,6 +19,7 @@ import ( "context" + "fmt" "testing" "time" @@ -207,6 +208,28 @@ func (suite *AccountTestSuite) TestDereferenceLocalAccountWithUnknownUserURI() { suite.Nil(fetchedAccount) } +func (suite *AccountTestSuite) TestDereferenceRemoteAccountWithNonMatchingURI() { + fetchingAccount := suite.testAccounts["local_account_1"] + + const ( + remoteURI = "https://turnip.farm/users/turniplover6969" + remoteAltURI = "https://turnip.farm/users/turniphater420" + ) + + // Create a copy of this remote account at alternative URI. + remotePerson := suite.client.TestRemotePeople[remoteURI] + suite.client.TestRemotePeople[remoteAltURI] = remotePerson + + // Attempt to fetch account at alternative URI, it should fail! + fetchedAccount, _, err := suite.dereferencer.GetAccountByURI( + context.Background(), + fetchingAccount.Username, + testrig.URLMustParse(remoteAltURI), + ) + suite.Equal(err.Error(), fmt.Sprintf("enrichAccount: dereferenced account uri %s does not match %s", remoteURI, remoteAltURI)) + suite.Nil(fetchedAccount) +} + func TestAccountTestSuite(t *testing.T) { suite.Run(t, new(AccountTestSuite)) } diff --git a/internal/federation/dereferencing/dereferencer_test.go b/internal/federation/dereferencing/dereferencer_test.go index 517479a50..c726467de 100644 --- a/internal/federation/dereferencing/dereferencer_test.go +++ b/internal/federation/dereferencing/dereferencer_test.go @@ -35,6 +35,7 @@ type DereferencerStandardTestSuite struct { db db.DB storage *storage.Driver state state.State + client *testrig.MockHTTPClient testRemoteStatuses map[string]vocab.ActivityStreamsNote testRemotePeople map[string]vocab.ActivityStreamsPerson @@ -72,11 +73,12 @@ func (suite *DereferencerStandardTestSuite) SetupTest() { converter, ) + suite.client = testrig.NewMockHTTPClient(nil, "../../../testrig/media") suite.storage = testrig.NewInMemoryStorage() suite.state.DB = suite.db suite.state.Storage = suite.storage media := testrig.NewTestMediaManager(&suite.state) - suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, testrig.NewMockHTTPClient(nil, "../../../testrig/media")), media) + suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, suite.client), media) testrig.StandardDBSetup(suite.db, nil) } diff --git a/internal/federation/dereferencing/finger.go b/internal/federation/dereferencing/finger.go index 514a058ba..1b3e915ba 100644 --- a/internal/federation/dereferencing/finger.go +++ b/internal/federation/dereferencing/finger.go @@ -21,9 +21,9 @@ "context" "encoding/json" "net/url" - "strings" apimodel "github.com/superseriousbusiness/gotosocial/internal/api/model" + apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/log" "github.com/superseriousbusiness/gotosocial/internal/transport" @@ -74,10 +74,12 @@ func (d *Dereferencer) fingerRemoteAccount( return "", nil, err } - _, accountDomain, err := util.ExtractWebfingerParts(resp.Subject) + accUsername, accDomain, err := util.ExtractWebfingerParts(resp.Subject) if err != nil { err = gtserror.Newf("error extracting subject parts for %s: %w", target, err) return "", nil, err + } else if accUsername != username { + return "", nil, gtserror.Newf("response username does not match input for %s: %w", target, err) } // Look through links for the first @@ -92,8 +94,7 @@ func (d *Dereferencer) fingerRemoteAccount( continue } - if !strings.EqualFold(link.Type, "application/activity+json") && - !strings.EqualFold(link.Type, "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"") { + if !apiutil.ASContentType(link.Type) { // Not an AP type, ignore. continue } @@ -121,7 +122,7 @@ func (d *Dereferencer) fingerRemoteAccount( } // All looks good, return happily! - return accountDomain, uri, nil + return accDomain, uri, nil } return "", nil, gtserror.Newf("no suitable self, AP-type link found in webfinger response for %s", target) diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go index 23c6e98c8..6d3dd5691 100644 --- a/internal/federation/dereferencing/status.go +++ b/internal/federation/dereferencing/status.go @@ -413,7 +413,7 @@ func (d *Dereferencer) enrichStatus( } // Ensure we have the author account of the status dereferenced (+ up-to-date). If this is a new status - // (i.e. status.AccountID == "") then any error here is irrecoverable. AccountID must ALWAYS be set. + // (i.e. status.AccountID == "") then any error here is irrecoverable. status.AccountID must ALWAYS be set. if _, _, err := d.getAccountByURI(ctx, requestUser, attributedTo); err != nil && status.AccountID == "" { return nil, nil, gtserror.Newf("failed to dereference status author %s: %w", uri, err) } @@ -425,11 +425,30 @@ func (d *Dereferencer) enrichStatus( return nil, nil, gtserror.Newf("error converting statusable to gts model for status %s: %w", uri, err) } + // Ensure final status isn't attempting + // to claim being authored by local user. + if latestStatus.Account.IsLocal() { + return nil, nil, gtserror.Newf( + "dereferenced status %s claiming to be local", + latestStatus.URI, + ) + } + + // Ensure the final parsed status URI / URL matches + // the input URI we fetched (or received) it as. + if expect := uri.String(); latestStatus.URI != expect && + latestStatus.URL != expect { + return nil, nil, gtserror.Newf( + "dereferenced status uri %s does not match %s", + latestStatus.URI, expect, + ) + } + + var isNew bool + // Based on the original provided // status model, determine whether // this is a new insert / update. - var isNew bool - if isNew = (status.ID == ""); isNew { // Generate new status ID from the provided creation date. diff --git a/internal/federation/dereferencing/status_test.go b/internal/federation/dereferencing/status_test.go index e9cdbcff5..2d0085cce 100644 --- a/internal/federation/dereferencing/status_test.go +++ b/internal/federation/dereferencing/status_test.go @@ -19,6 +19,7 @@ import ( "context" + "fmt" "testing" "github.com/stretchr/testify/suite" @@ -218,6 +219,28 @@ func (suite *StatusTestSuite) TestDereferenceStatusWithImageAndNoContent() { suite.NoError(err) } +func (suite *StatusTestSuite) TestDereferenceStatusWithNonMatchingURI() { + fetchingAccount := suite.testAccounts["local_account_1"] + + const ( + remoteURI = "https://turnip.farm/users/turniplover6969/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042" + remoteAltURI = "https://turnip.farm/users/turniphater420/statuses/70c53e54-3146-42d5-a630-83c8b6c7c042" + ) + + // Create a copy of this remote account at alternative URI. + remoteStatus := suite.client.TestRemoteStatuses[remoteURI] + suite.client.TestRemoteStatuses[remoteAltURI] = remoteStatus + + // Attempt to fetch account at alternative URI, it should fail! + fetchedStatus, _, err := suite.dereferencer.GetStatusByURI( + context.Background(), + fetchingAccount.Username, + testrig.URLMustParse(remoteAltURI), + ) + suite.Equal(err.Error(), fmt.Sprintf("enrichStatus: dereferenced status uri %s does not match %s", remoteURI, remoteAltURI)) + suite.Nil(fetchedStatus) +} + func TestStatusTestSuite(t *testing.T) { suite.Run(t, new(StatusTestSuite)) } diff --git a/internal/federation/federatingactor.go b/internal/federation/federatingactor.go index b91165bb1..bf54962db 100644 --- a/internal/federation/federatingactor.go +++ b/internal/federation/federatingactor.go @@ -23,70 +23,18 @@ "fmt" "net/http" "net/url" - "strings" errorsv2 "codeberg.org/gruf/go-errors/v2" "codeberg.org/gruf/go-kv" "github.com/superseriousbusiness/activity/pub" "github.com/superseriousbusiness/activity/streams/vocab" "github.com/superseriousbusiness/gotosocial/internal/ap" + apiutil "github.com/superseriousbusiness/gotosocial/internal/api/util" "github.com/superseriousbusiness/gotosocial/internal/db" "github.com/superseriousbusiness/gotosocial/internal/gtserror" "github.com/superseriousbusiness/gotosocial/internal/log" ) -// IsASMediaType will return whether the given content-type string -// matches one of the 2 possible ActivityStreams incoming content types: -// - application/activity+json -// - application/ld+json;profile=https://w3.org/ns/activitystreams -// -// Where for the above we are leniant with whitespace, quotes, and charset. -func IsASMediaType(ct string) bool { - var ( - // First content-type part, - // contains the application/... - p1 string = ct //nolint:revive - - // Second content-type part, - // contains AS IRI or charset - // if provided. - p2 string - ) - - // Split content-type by semi-colon. - sep := strings.IndexByte(ct, ';') - if sep >= 0 { - p1 = ct[:sep] - - // Trim all start/end - // space of second part. - p2 = ct[sep+1:] - p2 = strings.Trim(p2, " ") - } - - // Trim any ending space from the - // main content-type part of string. - p1 = strings.TrimRight(p1, " ") - - switch p1 { - case "application/activity+json": - // Accept with or without charset. - // This should be case insensitive. - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Type#charset - return p2 == "" || strings.EqualFold(p2, "charset=utf-8") - - case "application/ld+json": - // Drop any quotes around the URI str. - p2 = strings.ReplaceAll(p2, "\"", "") - - // End part must be a ref to the main AS namespace IRI. - return p2 == "profile=https://www.w3.org/ns/activitystreams" - - default: - return false - } -} - // federatingActor wraps the pub.FederatingActor // with some custom GoToSocial-specific logic. type federatingActor struct { @@ -124,7 +72,7 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr // Ensure valid ActivityPub Content-Type. // https://www.w3.org/TR/activitypub/#server-to-server-interactions - if ct := r.Header.Get("Content-Type"); !IsASMediaType(ct) { + if ct := r.Header.Get("Content-Type"); !apiutil.ASContentType(ct) { const ct1 = "application/activity+json" const ct2 = "application/ld+json;profile=https://w3.org/ns/activitystreams" err := fmt.Errorf("Content-Type %s not acceptable, this endpoint accepts: [%q %q]", ct, ct1, ct2) diff --git a/internal/federation/federatingactor_test.go b/internal/federation/federatingactor_test.go index d07b56537..b6814862f 100644 --- a/internal/federation/federatingactor_test.go +++ b/internal/federation/federatingactor_test.go @@ -154,71 +154,3 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() { func TestFederatingActorTestSuite(t *testing.T) { suite.Run(t, new(FederatingActorTestSuite)) } - -func TestIsASMediaType(t *testing.T) { - for _, test := range []struct { - Input string - Expect bool - }{ - { - Input: "application/activity+json", - Expect: true, - }, - { - Input: "application/activity+json; charset=utf-8", - Expect: true, - }, - { - Input: "application/activity+json;charset=utf-8", - Expect: true, - }, - { - Input: "application/activity+json ;charset=utf-8", - Expect: true, - }, - { - Input: "application/activity+json ; charset=utf-8", - Expect: true, - }, - { - Input: "application/ld+json;profile=https://www.w3.org/ns/activitystreams", - Expect: true, - }, - { - Input: "application/ld+json;profile=\"https://www.w3.org/ns/activitystreams\"", - Expect: true, - }, - { - Input: "application/ld+json ;profile=https://www.w3.org/ns/activitystreams", - Expect: true, - }, - { - Input: "application/ld+json ;profile=\"https://www.w3.org/ns/activitystreams\"", - Expect: true, - }, - { - Input: "application/ld+json ; profile=https://www.w3.org/ns/activitystreams", - Expect: true, - }, - { - Input: "application/ld+json ; profile=\"https://www.w3.org/ns/activitystreams\"", - Expect: true, - }, - { - Input: "application/ld+json; profile=https://www.w3.org/ns/activitystreams", - Expect: true, - }, - { - Input: "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"", - Expect: true, - }, - { - Input: "application/ld+json", - Expect: false, - }, - } { - if federation.IsASMediaType(test.Input) != test.Expect { - t.Errorf("did not get expected result %v for input: %s", test.Expect, test.Input) - } - } -} diff --git a/internal/transport/dereference.go b/internal/transport/dereference.go index e1702f9f4..3a33a81ad 100644 --- a/internal/transport/dereference.go +++ b/internal/transport/dereference.go @@ -64,9 +64,16 @@ func (t *transport) Dereference(ctx context.Context, iri *url.URL) ([]byte, erro } defer rsp.Body.Close() + // Ensure a non-error status response. if rsp.StatusCode != http.StatusOK { return nil, gtserror.NewFromResponse(rsp) } + // Ensure that the incoming request content-type is expected. + if ct := rsp.Header.Get("Content-Type"); !apiutil.ASContentType(ct) { + err := gtserror.Newf("non activity streams response: %s", ct) + return nil, gtserror.SetMalformed(err) + } + return io.ReadAll(rsp.Body) } diff --git a/internal/transport/derefinstance.go b/internal/transport/derefinstance.go index c6572b727..439c5ae23 100644 --- a/internal/transport/derefinstance.go +++ b/internal/transport/derefinstance.go @@ -101,10 +101,17 @@ func dereferenceByAPIV1Instance(ctx context.Context, t *transport, iri *url.URL) } defer resp.Body.Close() + // Ensure a non-error status response. if resp.StatusCode != http.StatusOK { return nil, gtserror.NewFromResponse(resp) } + // Ensure that the incoming request content-type is expected. + if ct := resp.Header.Get("Content-Type"); !apiutil.JSONContentType(ct) { + err := gtserror.Newf("non json response type: %s", ct) + return nil, gtserror.SetMalformed(err) + } + b, err := io.ReadAll(resp.Body) if err != nil { return nil, err @@ -251,20 +258,27 @@ func callNodeInfoWellKnown(ctx context.Context, t *transport, iri *url.URL) (*ur } defer resp.Body.Close() + // Ensure a non-error status response. if resp.StatusCode != http.StatusOK { return nil, gtserror.NewFromResponse(resp) } + // Ensure that the incoming request content-type is expected. + if ct := resp.Header.Get("Content-Type"); !apiutil.JSONContentType(ct) { + err := gtserror.Newf("non json response type: %s", ct) + return nil, gtserror.SetMalformed(err) + } + b, err := io.ReadAll(resp.Body) if err != nil { return nil, err } else if len(b) == 0 { - return nil, errors.New("callNodeInfoWellKnown: response bytes was len 0") + return nil, gtserror.New("response bytes was len 0") } wellKnownResp := &apimodel.WellKnownResponse{} if err := json.Unmarshal(b, wellKnownResp); err != nil { - return nil, fmt.Errorf("callNodeInfoWellKnown: could not unmarshal server response as WellKnownResponse: %s", err) + return nil, gtserror.Newf("could not unmarshal server response as WellKnownResponse: %w", err) } // look through the links for the first one that matches the nodeinfo schema, this is what we need @@ -275,11 +289,11 @@ func callNodeInfoWellKnown(ctx context.Context, t *transport, iri *url.URL) (*ur } nodeinfoHref, err = url.Parse(l.Href) if err != nil { - return nil, fmt.Errorf("callNodeInfoWellKnown: couldn't parse url %s: %s", l.Href, err) + return nil, gtserror.Newf("couldn't parse url %s: %w", l.Href, err) } } if nodeinfoHref == nil { - return nil, errors.New("callNodeInfoWellKnown: could not find nodeinfo rel in well known response") + return nil, gtserror.New("could not find nodeinfo rel in well known response") } return nodeinfoHref, nil @@ -302,20 +316,27 @@ func callNodeInfo(ctx context.Context, t *transport, iri *url.URL) (*apimodel.No } defer resp.Body.Close() + // Ensure a non-error status response. if resp.StatusCode != http.StatusOK { return nil, gtserror.NewFromResponse(resp) } + // Ensure that the incoming request content-type is expected. + if ct := resp.Header.Get("Content-Type"); !apiutil.NodeInfo2ContentType(ct) { + err := gtserror.Newf("non nodeinfo schema 2.0 response: %s", ct) + return nil, gtserror.SetMalformed(err) + } + b, err := io.ReadAll(resp.Body) if err != nil { return nil, err } else if len(b) == 0 { - return nil, errors.New("callNodeInfo: response bytes was len 0") + return nil, gtserror.New("response bytes was len 0") } niResp := &apimodel.Nodeinfo{} if err := json.Unmarshal(b, niResp); err != nil { - return nil, fmt.Errorf("callNodeInfo: could not unmarshal server response as Nodeinfo: %s", err) + return nil, gtserror.Newf("could not unmarshal server response as Nodeinfo: %w", err) } return niResp, nil diff --git a/internal/transport/finger.go b/internal/transport/finger.go index 385af5e1c..9bcb0fa7e 100644 --- a/internal/transport/finger.go +++ b/internal/transport/finger.go @@ -98,9 +98,17 @@ func (t *transport) Finger(ctx context.Context, targetUsername string, targetDom // again here to renew the TTL t.controller.state.Caches.GTS.Webfinger.Set(targetDomain, url) } + if rsp.StatusCode == http.StatusGone { return nil, fmt.Errorf("account has been deleted/is gone") } + + // Ensure that the incoming request content-type is expected. + if ct := rsp.Header.Get("Content-Type"); !apiutil.JSONJRDContentType(ct) { + err := gtserror.Newf("non webfinger type response: %s", ct) + return nil, gtserror.SetMalformed(err) + } + return io.ReadAll(rsp.Body) } @@ -193,6 +201,12 @@ func (t *transport) webfingerFromHostMeta(ctx context.Context, targetDomain stri return "", fmt.Errorf("GET request for %s failed: %s", req.URL.String(), rsp.Status) } + // Ensure that the incoming request content-type is expected. + if ct := rsp.Header.Get("Content-Type"); !apiutil.XMLXRDContentType(ct) { + err := gtserror.Newf("non host-meta type response: %s", ct) + return "", gtserror.SetMalformed(err) + } + e := xml.NewDecoder(rsp.Body) var hm apimodel.HostMeta if err := e.Decode(&hm); err != nil { diff --git a/testrig/transportcontroller.go b/testrig/transportcontroller.go index 46a9b0fb2..b32a0d804 100644 --- a/testrig/transportcontroller.go +++ b/testrig/transportcontroller.go @@ -245,9 +245,7 @@ func NewMockHTTPClient(do func(req *http.Request) (*http.Response, error), relat StatusCode: responseCode, Body: readCloser, ContentLength: int64(responseContentLength), - Header: http.Header{ - "content-type": {responseContentType}, - }, + Header: http.Header{"Content-Type": {responseContentType}}, }, nil }