[bugfix] check remote status permissibility (#2703)

* add more stringent checks for remote status permissibility

* add check for inreplyto of a remote status being a boost

* do not permit inReplyTo boost wrapper statuses

* change comment wording

* fix calls to NewFederator()

* add code comments for NotPermitted() and SetNotPermitted()

* improve comment

* check that existing != nil before attempting delete

* ensure replying account isn't suspended

* use a debug log instead of info. check for boost using ID

* shorten log string length. make info level

* add note that replying to boost wrapper status shouldn't be able to happen anyways

* update to use onFail() function

5 files changed, 122 insertions, 8 deletions

diff --git a/internal/federation/dereferencing/dereferencer.go b/internal/federation/dereferencing/dereferencer.go
index f4596935e..24e579408 100644
--- a/internal/federation/dereferencing/dereferencer.go
+++ b/internal/federation/dereferencing/dereferencer.go
@@ -22,6 +22,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/superseriousbusiness/gotosocial/internal/filter/visibility"
 	"github.com/superseriousbusiness/gotosocial/internal/media"
 	"github.com/superseriousbusiness/gotosocial/internal/state"
 	"github.com/superseriousbusiness/gotosocial/internal/transport"
@@ -72,6 +73,7 @@ type Dereferencer struct {
 	converter           *typeutils.Converter
 	transportController transport.Controller
 	mediaManager        *media.Manager
+	visibility          *visibility.Filter
 
 	// all protected by State{}.FedLocks.
 	derefAvatars map[string]*media.ProcessingMedia
@@ -87,6 +89,7 @@ func NewDereferencer(
 	state *state.State,
 	converter *typeutils.Converter,
 	transportController transport.Controller,
+	visFilter *visibility.Filter,
 	mediaManager *media.Manager,
 ) Dereferencer {
 	return Dereferencer{
@@ -94,6 +97,7 @@ func NewDereferencer(
 		converter:           converter,
 		transportController: transportController,
 		mediaManager:        mediaManager,
+		visibility:          visFilter,
 		derefAvatars:        make(map[string]*media.ProcessingMedia),
 		derefHeaders:        make(map[string]*media.ProcessingMedia),
 		derefEmojis:         make(map[string]*media.ProcessingEmoji),
diff --git a/internal/federation/dereferencing/dereferencer_test.go b/internal/federation/dereferencing/dereferencer_test.go
index 7f79e8ac0..293118167 100644
--- a/internal/federation/dereferencing/dereferencer_test.go
+++ b/internal/federation/dereferencing/dereferencer_test.go
@@ -77,8 +77,10 @@ func (suite *DereferencerStandardTestSuite) SetupTest() {
 	suite.storage = testrig.NewInMemoryStorage()
 	suite.state.DB = suite.db
 	suite.state.Storage = suite.storage
+
+	visFilter := visibility.NewFilter(&suite.state)
 	media := testrig.NewTestMediaManager(&suite.state)
-	suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, suite.client), media)
+	suite.dereferencer = dereferencing.NewDereferencer(&suite.state, converter, testrig.NewTestTransportController(&suite.state, suite.client), visFilter, media)
 	testrig.StandardDBSetup(suite.db, nil)
 }
 
diff --git a/internal/federation/dereferencing/status.go b/internal/federation/dereferencing/status.go
index 397d2aa28..c8012178c 100644
--- a/internal/federation/dereferencing/status.go
+++ b/internal/federation/dereferencing/status.go
@@ -503,9 +503,16 @@ func (d *Dereferencer) enrichStatus(
 	latestStatus.FetchedAt = time.Now()
 	latestStatus.Local = status.Local
 
-	// Ensure the status' poll remains consistent, else reset the poll.
-	if err := d.fetchStatusPoll(ctx, status, latestStatus); err != nil {
-		return nil, nil, gtserror.Newf("error populating poll for status %s: %w", uri, err)
+	// Check if this is a permitted status we should accept.
+	permit, err := d.isPermittedStatus(ctx, status, latestStatus)
+	if err != nil {
+		return nil, nil, gtserror.Newf("error checking permissibility for status %s: %w", uri, err)
+	}
+
+	if !permit {
+		// Return a checkable error type that can be ignored.
+		err := gtserror.Newf("dropping unpermitted status: %s", uri)
+		return nil, nil, gtserror.SetNotPermitted(err)
 	}
 
 	// Ensure the status' mentions are populated, and pass in existing to check for changes.
@@ -513,6 +520,11 @@ func (d *Dereferencer) enrichStatus(
 		return nil, nil, gtserror.Newf("error populating mentions for status %s: %w", uri, err)
 	}
 
+	// Ensure the status' poll remains consistent, else reset the poll.
+	if err := d.fetchStatusPoll(ctx, status, latestStatus); err != nil {
+		return nil, nil, gtserror.Newf("error populating poll for status %s: %w", uri, err)
+	}
+
 	// Now that we know who this status replies to (handled by ASStatusToStatus)
 	// and who it mentions, we can add a ThreadID to it if necessary.
 	if err := d.threadStatus(ctx, latestStatus); err != nil {
@@ -550,6 +562,84 @@ func (d *Dereferencer) enrichStatus(
 	return latestStatus, apubStatus, nil
 }
 
+// isPermittedStatus returns whether the given status
+// is permitted to be stored on this instance, checking
+// whether the author is suspended, and passes visibility
+// checks against status being replied-to (if any).
+func (d *Dereferencer) isPermittedStatus(
+	ctx context.Context,
+	existing *gtsmodel.Status,
+	status *gtsmodel.Status,
+) (
+	permitted bool, // is permitted?
+	err error,
+) {
+
+	// our failure condition handling
+	// at the end of this function for
+	// the case of permission = false.
+	onFail := func() (bool, error) {
+		if existing != nil {
+			log.Infof(ctx, "deleting unpermitted: %s", existing.URI)
+
+			// Delete existing status from database as it's no longer permitted.
+			if err := d.state.DB.DeleteStatusByID(ctx, existing.ID); err != nil {
+				log.Errorf(ctx, "error deleting %s after permissivity fail: %v", existing.URI, err)
+			}
+		}
+		return false, nil
+	}
+
+	if !status.Account.SuspendedAt.IsZero() {
+		// The status author is suspended,
+		// this shouldn't have reached here
+		// but it's a fast check anyways.
+		return onFail()
+	}
+
+	if status.InReplyToURI == "" {
+		// This status isn't in
+		// reply to anything!
+		return true, nil
+	}
+
+	if status.InReplyTo == nil {
+		// If no inReplyTo has been set,
+		// we return here for now as we
+		// can't perform further checks.
+		//
+		// Worst case we allow something
+		// through, and later on during
+		// refetch it will get deleted.
+		return true, nil
+	}
+
+	if status.InReplyTo.BoostOfID != "" {
+		// We do not permit replies to
+		// boost wrapper statuses. (this
+		// shouldn't be able to happen).
+		return onFail()
+	}
+
+	// Check visibility of inReplyTo to status author.
+	permitted, err = d.visibility.StatusVisible(ctx,
+		status.Account,
+		status.InReplyTo,
+	)
+	if err != nil {
+		return false, gtserror.Newf("error checking in-reply-to visibility: %w", err)
+	}
+
+	if permitted &&
+		*status.InReplyTo.Replyable {
+		// This status is visible AND
+		// replyable, in this economy?!
+		return true, nil
+	}
+
+	return onFail()
+}
+
 // populateMentionTarget tries to populate the given
 // mention with the correct TargetAccount and (if not
 // yet set) TargetAccountURI, returning the populated
diff --git a/internal/federation/federatingactor_test.go b/internal/federation/federatingactor_test.go
index b6814862f..0c805a2c6 100644
--- a/internal/federation/federatingactor_test.go
+++ b/internal/federation/federatingactor_test.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/stretchr/testify/suite"
 	"github.com/superseriousbusiness/gotosocial/internal/federation"
+	"github.com/superseriousbusiness/gotosocial/internal/filter/visibility"
 	"github.com/superseriousbusiness/gotosocial/internal/gtsmodel"
 	"github.com/superseriousbusiness/gotosocial/internal/util"
 	"github.com/superseriousbusiness/gotosocial/testrig"
@@ -60,14 +61,21 @@ func (suite *FederatingActorTestSuite) TestSendNoRemoteFollowers() {
 	tc := testrig.NewTestTransportController(&suite.state, httpClient)
 
 	// setup module being tested
-	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.typeconverter, testrig.NewTestMediaManager(&suite.state))
+	federator := federation.NewFederator(
+		&suite.state,
+		testrig.NewTestFederatingDB(&suite.state),
+		tc,
+		suite.typeconverter,
+		visibility.NewFilter(&suite.state),
+		testrig.NewTestMediaManager(&suite.state),
+	)
 
 	activity, err := federator.FederatingActor().Send(ctx, testrig.URLMustParse(testAccount.OutboxURI), testActivity)
 	suite.NoError(err)
 	suite.NotNil(activity)
 
 	// because zork has no remote followers, sent messages should be empty (no messages sent to own instance)
-	suite.Empty(httpClient.SentMessages)
+	suite.Empty(&httpClient.SentMessages)
 }
 
 func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {
@@ -105,8 +113,16 @@ func (suite *FederatingActorTestSuite) TestSendRemoteFollower() {
 
 	httpClient := testrig.NewMockHTTPClient(nil, "../../testrig/media")
 	tc := testrig.NewTestTransportController(&suite.state, httpClient)
+
 	// setup module being tested
-	federator := federation.NewFederator(&suite.state, testrig.NewTestFederatingDB(&suite.state), tc, suite.typeconverter, testrig.NewTestMediaManager(&suite.state))
+	federator := federation.NewFederator(
+		&suite.state,
+		testrig.NewTestFederatingDB(&suite.state),
+		tc,
+		suite.typeconverter,
+		visibility.NewFilter(&suite.state),
+		testrig.NewTestMediaManager(&suite.state),
+	)
 
 	activity, err := federator.FederatingActor().Send(ctx, testrig.URLMustParse(testAccount.OutboxURI), testActivity)
 	suite.NoError(err)
diff --git a/internal/federation/federator.go b/internal/federation/federator.go
index 8377546a1..f97d73cf8 100644
--- a/internal/federation/federator.go
+++ b/internal/federation/federator.go
@@ -22,6 +22,7 @@ import (
 	"github.com/superseriousbusiness/gotosocial/internal/db"
 	"github.com/superseriousbusiness/gotosocial/internal/federation/dereferencing"
 	"github.com/superseriousbusiness/gotosocial/internal/federation/federatingdb"
+	"github.com/superseriousbusiness/gotosocial/internal/filter/visibility"
 	"github.com/superseriousbusiness/gotosocial/internal/media"
 	"github.com/superseriousbusiness/gotosocial/internal/state"
 	"github.com/superseriousbusiness/gotosocial/internal/transport"
@@ -50,6 +51,7 @@ func NewFederator(
 	federatingDB federatingdb.DB,
 	transportController transport.Controller,
 	converter *typeutils.Converter,
+	visFilter *visibility.Filter,
 	mediaManager *media.Manager,
 ) *Federator {
 	clock := &Clock{}
@@ -60,7 +62,7 @@ func NewFederator(
 		converter:           converter,
 		transportController: transportController,
 		mediaManager:        mediaManager,
-		Dereferencer:        dereferencing.NewDereferencer(state, converter, transportController, mediaManager),
+		Dereferencer:        dereferencing.NewDereferencer(state, converter, transportController, visFilter, mediaManager),
 	}
 	actor := newFederatingActor(f, f, federatingDB, clock)
 	f.actor = actor