[bugfix] Don't return 500 when someone tries to Delete or Update something they don't own (#4663)
# Description
> If this is a code change, please include a summary of what you've coded, and link to the issue(s) it closes/implements.
>
> If this is a documentation change, please briefly describe what you've changed and why.
Noticed Friendica doing a weird thing where they're sending out deletes of someone else's *tombstone* (?):
```json
{
"@context": [
"https://www.w3.org/ns/activitystreams",
"https://w3id.org/security/v1",
{
"Hashtag": "as:Hashtag",
"PropertyValue": "schema:PropertyValue",
"conversation": "ostatus:conversation",
"dfrn": "http://purl.org/macgirvin/dfrn/1.0/",
"diaspora": "https://diasporafoundation.org/ns/",
"directMessage": "litepub:directMessage",
"discoverable": "toot:discoverable",
"featured": {
"@id": "toot:featured",
"@type": "@id"
},
"litepub": "http://litepub.social/ns#",
"manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
"ostatus": "http://ostatus.org#",
"quoteUrl": "as:quoteUrl",
"schema": "http://schema.org#",
"sensitive": "as:sensitive",
"toot": "http://joinmastodon.org/ns#",
"value": "schema:value",
"vcard": "http://www.w3.org/2006/vcard/ns#"
}
],
"actor": "https://sfba.social/users/godzero",
"cc": "https://sfba.social/users/godzero/followers",
"id": "https://thegoatery.dyndns.org/objects/01e8a30d-e643609307c99234-626ae548/Delete",
"instrument": {
"id": "https://thegoatery.dyndns.org/friendica",
"name": "Friendica 'Interrupted Fern' 2024.12-1576",
"type": "Application",
"url": "https://thegoatery.dyndns.org"
},
"object": {
"id": "https://sfba.social/users/godzero/statuses/111846802224346241",
"type": "Tombstone"
},
"published": "2024-01-30T20:32:42Z",
"to": [
"https://thegoatery.dyndns.org/profile/goatsarah",
"https://www.w3.org/ns/activitystreams#Public"
],
"type": "Delete"
}
```
This was triggering an error in the activity library which annoyingly isn't wrapped in an error type, but we can catch it by checking the error string and just returning 403 if we see it, instead of 500. It's not actually an internal server error after all.
## Checklist
Please put an x inside each checkbox to indicate that you've read and followed it: `[ ]` -> `[x]`
If this is a documentation change, only the first two checkboxes must be filled (you can delete the others if you want).
- [x] I/we have read the [GoToSocial contribution guidelines](https://codeberg.org/superseriousbusiness/gotosocial/src/branch/main/CONTRIBUTING.md).
- [x] I/we have not used so-called 'AI' to create the proposed changes.
- [x] I/we have discussed the proposed changes already, either in an issue on the repository, or in the Matrix chat.
- [x] I/we have performed a self-review of added code.
- [x] I/we have written code that is legible and maintainable by others.
- [x] I/we have commented the added code, particularly in hard-to-understand areas.
- [ ] I/we have made any necessary changes to documentation.
- [ ] I/we have added tests that cover new code.
- [x] I/we have run tests and they pass locally with the changes.
- [x] I/we have run `go fmt ./...` and `golangci-lint run`.
Reviewed-on: https://codeberg.org/superseriousbusiness/gotosocial/pulls/4663
Co-authored-by: tobi <tobi.smethurst@protonmail.com>
Co-committed-by: tobi <tobi.smethurst@protonmail.com>
This commit is contained in:
@@ -311,7 +311,8 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr
|
||||
// "cannot determine id of activitystreams value" from activity/pub/util.go
|
||||
// This likely means we've been delivered a type we just don't recognise.
|
||||
// If this is so, just log it and return `false, nil` so caller gets 202.
|
||||
if strings.Contains(err.Error(), "cannot determine id of activitystreams") {
|
||||
errString := err.Error()
|
||||
if strings.Contains(errString, "cannot determine id of activitystreams") {
|
||||
var l = "ignored unhandleable Activity posted to inbox"
|
||||
if b != nil {
|
||||
l += ": " + string(b)
|
||||
@@ -320,6 +321,23 @@ func (f *federatingActor) PostInboxScheme(ctx context.Context, w http.ResponseWr
|
||||
return false, nil
|
||||
}
|
||||
|
||||
// Check for error "object [blah] not in activity origin" from
|
||||
// activity/pub/util.go. This means someone is trying to send
|
||||
// an Activity that updates or deletes an object that doesn't
|
||||
// belong to them, eg., `@someone@example.org` is trying to
|
||||
// Delete or Update an object on a different server.
|
||||
if strings.Contains(errString, "not in activity origin") {
|
||||
const text = "actor not permitted to delete or update object that doesn't belong to them"
|
||||
if b != nil {
|
||||
// Log the object so we can
|
||||
// keep track of these things.
|
||||
log.Warnf(ctx, text+": "+string(b))
|
||||
}
|
||||
|
||||
// Tell the remote server they're not allowed to do that.
|
||||
return false, gtserror.NewErrorForbidden(errors.New(text), text)
|
||||
}
|
||||
|
||||
// Something else went wrong, what the heck!
|
||||
// This is an actual 500-able error.
|
||||
var wrappedErr error
|
||||
|
||||
Reference in New Issue
Block a user