Commit graph

6748 commits

Author SHA1 Message Date
Floatingghost b72127b45a Merge remote-tracking branch 'oneric-sec/media-owner' into develop 2024-05-22 19:36:10 +01:00
Oneric 9a91299f96 Don't try to handle non-media objects as media
Trying to display non-media as media crashed the renderer,
but when posting a status with a valid, non-media object id
the post was still created, but then crashed e.g. timeline rendering.
It also crashed C2S inbox reads, so this could not be used to leak
private posts.
2024-05-22 20:30:23 +02:00
Oneric 0c2b33458d Restrict media usage to owners
In Mastodon media can only be used by owners and only be associated with
a single post. We currently allow media to be associated with several
posts and until now did not limit their usage in posts to media owners.
However, media update and GET lookup was already limited to owners.
(In accordance with allowing media reuse, we also still allow GET
lookups of media already used in a post unlike Mastodon)

Allowing reuse isn’t problematic per se, but allowing use by non-owners
can be problematic if media ids of private-scoped posts can be guessed
since creating a new post with this media id will reveal the uploaded
file content and alt text.
Given media ids are currently just part of a sequentieal series shared
with some other objects, guessing media ids is with some persistence
indeed feasible.

E.g. sampline some public media ids from a real-world
instance with 112 total and 61 monthly-active users:

  17.465.096  at  t0
  17.472.673  at  t1 = t0 + 4h
  17.473.248  at  t2 = t1 + 20min

This gives about 30 new ids per minute of which most won't be
local media but remote and local posts, poll answers etc.
Assuming the default ratelimit of 15 post actions per 10s, scraping all
media for the 4h interval takes about 84 minutes and scraping the 20min
range mere 6.3 minutes. (Until the preceding commit, post updates were
not rate limited at all, allowing even faster scraping.)
If an attacker can infer (e.g. via reply to a follower-only post not
accessbile to the attacker) some sensitive information was uploaded
during a specific time interval and has some pointers regarding the
nature of the information, identifying the specific upload out of all
scraped media for this timerange is not impossible.

Thus restrict media usage to owners.

Checking ownership just in ActivitDraft would already be sufficient,
since when a scheduled status actually gets posted it goes through
ActivityDraft again, but would erroneously return a success status
when scheduling an illegal post.

Independently discovered and fixed by mint in Pleroma
1afde067b1
2024-05-22 20:30:18 +02:00
marcin mikołajczak 3a21293970 Fix tests
Signed-off-by: marcin mikołajczak <git@mkljczk.pl>
2024-05-22 19:27:31 +01:00
marcin mikołajczak 0d66237205 Fix validate_webfinger when running a different domain for Webfinger
Signed-off-by: marcin mikołajczak <git@mkljczk.pl>
2024-05-22 19:20:02 +01:00
Oneric 6ef6b2a289 Apply rate limits to status updates 2024-05-22 20:18:08 +02:00
Oneric 94e9c8f48a Purge unused media description update on post
In MastoAPI media descriptions are updated via the
media update API not upon post creation or post update.

This functionality was originally added about 6 years ago in
ba93396649 which was part of
https://git.pleroma.social/pleroma/pleroma/-/merge_requests/626 and
https://git.pleroma.social/pleroma/pleroma-fe/-/merge_requests/450.
They introduced image descriptions to the front- and backend,
but predate adoption of Mastodon API.

For a while adding an `descriptions` array on post creation might have
continued to work as an undocumented Pleroma extension to Masto API, but
at latest when OpenAPI specs were added for those endpoints four years
ago in 7803a85d2c, these codepaths ceased
to be used. The API specs don’t list a `descriptions` parameter and
any unknown parameters are stripped out.

The attachments_from_ids function is only called from
ScheduledActivity and ActivityDraft.create with the latter
only being called by CommonAPI.{post,update} whihc in turn
are only called from ScheduledActivity again, MastoAPI controller
and without any attachment or description parameter WelcomeMessage.
Therefore no codepath can contain a descriptions parameter.
2024-05-22 20:18:08 +02:00
Oneric 873aa9da1c activity_draft: mark new/2 as private 2024-05-22 20:18:08 +02:00
Alex Gleason a953b1d927 Prevent spoofing webfinger 2024-05-22 19:08:37 +01:00
Floatingghost f531484063 Merge branch 'develop' into backoff-http 2024-04-26 19:06:18 +01:00
FloatingGhost ad7dcf38a8 Add HTTP backoff cache to respect 429s 2024-04-26 19:00:35 +01:00
Oneric b0a46c1e2e Normalise public adressing to fix federation
Due to JSON-LD compaction the full address of public scope
may also occur in shorter forms and the spec requires us to treat them
all equivalently. To save us the pain of repeatedly checking for all
variants internally, normalise inbound data to just one form.
See note at: https://www.w3.org/TR/activitypub/#public-addressing

This needs to happen very early, even before the other addressing fixes
else an earlier validator will reject the object. This in turn required
to move the list-tpye normalisation earlier as well, but since I was
unsure about putting empty lists into the data when no such field
existed before, I excluded this case and thus the later fixing had to be
kept as well.

Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/670
2024-04-25 18:45:16 +02:00
floatingghost b1c6621e66 Merge pull request 'Read image description from EXIF data' (#744) from timorl/akkoma:elseinspe into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/744
2024-04-25 12:52:31 +00:00
floatingghost 764dbeded4 Merge pull request 'Accept all standard actor types' (#751) from Oneric/akkoma:all-actor-types into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/751
2024-04-24 17:09:02 +00:00
floatingghost 80e1c094c7 Merge pull request 'Don't strip newlines in pre' (#709) from snan/akkoma:pre into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/709
2024-04-24 17:00:34 +00:00
Oneric 83f75c3e93 Accept all standard actor types 2024-04-23 18:14:34 +02:00
Floatingghost 92168fa5a1 Merge remote-tracking branch 'origin/develop' into who-wants-to-yeet-c2s-i-want-to-yeet-c2s 2024-04-23 14:37:05 +01:00
Floatingghost 3e199242b0 remove upload_media from AP representation 2024-04-23 14:35:52 +01:00
timorl 9da0fe930e
Format, but this time with a non-ancient version of elixir 2024-04-19 18:07:50 +02:00
timorl 2a9db73b4c
Merge branch 'develop' into elseinspe 2024-04-19 17:11:55 +02:00
Floatingghost 1ed975636b Keep READ endpoints, purge WRITE 2024-04-19 11:06:01 +01:00
Floatingghost ddb8a5ef73 yeet AP C2S support
literally nothing uses C2S AP, and it's another route into core
systems which requires analysis and maintenance. A second API
is just extra surface for potentially bad things so let's take
it out back and obliterate it
2024-04-16 13:55:03 +01:00
Floatingghost 123db1abc4 Merge branch 'develop' into failed-fetch-processing 2024-04-16 12:35:54 +01:00
Floatingghost b2c29527fb make xmerl shut up about markup 2024-04-16 10:19:30 +01:00
timorl 59d32c10d9
Formatting 2024-04-16 08:02:13 +02:00
Floatingghost 2fc25980d1 fix pattern matching in fetch errors 2024-04-13 23:55:26 +01:00
floatingghost c1f0b6b875 Merge pull request 'Accept body parameters for /api/pleroma/notification_settings' (#738) from Oneric/akkoma:notif-setting-parameters into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/738
2024-04-13 22:55:02 +00:00
Floatingghost 49ed27cd96 require logger 2024-04-13 22:25:31 +01:00
Mark Felder d69cba1b93 Remove duplicate log messages from Transmogrifier
Object fetch errors are logged in the fetcher module
2024-04-12 20:31:31 +01:00
Mark Felder eeed051a0f Fix detection of user follower collection being private
We were overzealous with matching on a raw error from the object fetch that should have never been relied on like this. If we can't fetch successfully we should assume that the collection is private.

Building a more expressive and universal error struct to match on may be something to consider.
2024-04-12 20:29:11 +01:00
floatingghost 6f3c955aa0 Merge pull request 'elixir1.16 testing' (#742) from elixir1.16 into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/742
2024-04-12 18:49:33 +00:00
floatingghost 024ffadd80 Merge pull request 'Don't list old accounts as aliases in WebFinger' (#713) from erincandescent/akkoma:no-old-account-alias into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/713
2024-04-12 18:34:14 +00:00
Floatingghost df25d86999 Cleaned up FEP-fffd commits a bit 2024-04-12 18:50:57 +01:00
floatingghost 4887df12d7 Merge pull request 'Allow for url to be a list' (#718) from helge/akkoma:develop into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/718
2024-04-12 17:39:38 +00:00
floatingghost e6ca2b4d2a Merge pull request 'Fix array-less EmojiReacts' (#739) from Oneric/akkoma:tag-arrayless into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/739
2024-04-12 17:26:07 +00:00
floatingghost 6ba80aaff5 Merge pull request 'Check if data is visible before embedding it in OG tags' (#741) from ograph-restrictions into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/741
2024-04-12 17:22:59 +00:00
floatingghost 8e60177466 Merge pull request 'MRF.InlineQuotePolicy: Add link to post URL, not ID' (#733) from erincandescent/akkoma:quote-url into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/733
2024-04-12 17:02:52 +00:00
Erin Shepherd 75d9e2b375 MRF.InlineQuotePolicy: Add link to post URL, not ID
"id" is used for the canonical link to the AS2 representation of an object.
"url" is typically used for the canonical link to the HTTP representation.
It is what we use, for example, when following the "external source" link
in the frontend. However, it's not the link we include in the post contents
for quote posts.

Using URL instead means we include a more user-friendly URL for Mastodon,
and a working (in the browser) URL for Threads
2024-04-12 13:23:50 +02:00
Floatingghost 05f8179d08 check if data is visible before embedding it in OG tags
previously we would uncritically take data and format it into
tags for static-fe and the like - however, instances can be
configured to disallow unauthenticated access to these resources.

this means that OG tags as a vector for information leakage.

_technically_ this should only occur if you have both
restrict_unauthenticated *AND* you run static-fe, which makes no
sense since static-fe is for unauthenticated people in particular,
but hey ho.
2024-04-12 05:16:47 +01:00
Oneric bd74ad9ce4 Accept body parameters for /api/pleroma/notification_settings
This brings it in line with its documentation and akkoma-fe’s
expectations. For backwards compatibility URL parameters are still
accept with lower priority. Unfortunately this means duplicating
parameters and descriptions in the API spec.

Usually Plug already pre-merges parameters from different sources into
the plain 'params' parameter which then gets forwarded by Phoenix.
However, OpenApiSpex 3.x prevents this; 4.x is set to change this
  https://github.com/open-api-spex/open_api_spex/issues/334
  https://github.com/open-api-spex/open_api_spex/issues/92

Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/691
Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/722
2024-04-09 04:11:28 +02:00
Oneric 462225880a Accept EmojiReacts with non-array tag
JSON-LD compaction strips the array since it’s just one object

Fixes: https://akkoma.dev/AkkomaGang/akkoma/issues/720
2024-04-09 04:04:16 +02:00
Oneric 9598137d32 Drop base_url special casing in test env
61621ebdbc already explicitly added
the uploader base url to config/test.exs and it reduces differences
from prod.
2024-04-07 00:20:12 +02:00
floatingghost 554f19a9ed Merge pull request 'Refresh Users much more aggressively when processing Move activities' (#714) from erincandescent/akkoma:move-bust-cache into develop
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/714
2024-04-03 10:03:14 +00:00
Erin Shepherd 464db9ea0b Don't list old accounts as aliases in WebFinger
Per the XRD specification:

> 2.4. Element <Alias>
>
> The <Alias> element contains a URI value that is an additional
> identifier for the resource described by the XRD. This value
> MUST be an absolute URI. The <Alias> element does not identify
> additional resources the XRD is describing, **but rather provides
> additional identifiers for the same resource.**

(http://docs.oasis-open.org/xri/xrd/v1.0/os/xrd-1.0-os.html#element.alias, emphasis mine)

In other words, the alias list is expected to link to things which are
not just semantically the same, but exactly the same. Old user accounts
don't do that

This change should not pose a compatibility issue: Mastodon does not
list old accounts here (See e1fcb02867/app/serializers/webfinger_serializer.rb (L12))

The use of as:alsoKnownAs is also not quite semantically right here
(see https://www.w3.org/TR/did-core/#dfn-alsoknownas, which defines
it to be used to refer to identifiers which are interchangable) but
that's what DID get for reusing a property definition that Mastodon
already squatted long before they got to it
2024-04-01 13:34:58 +02:00
Oneric 48b3a35793 Update user reference after fetch
Since we always followed redirects (and until recently allowed fuzzy id
matches), the ap_id of the received object might differ from the iniital
fetch url. This lead to us mistakenly trying to insert a new user with
the same nickname, ap_id, etc as an existing user (which will fail due
to uniqueness constraints) instead of updating the existing one.
2024-03-25 14:05:05 -01:00
Oneric f07eb4cb55 Sanity check fetched user data
In order to properly process incoming notes we need
to be able to map the key id back to an actor.
Also, check collections actually belong to the same server.

Key ids of Hubzilla and Bridgy samples were updated to what
modern versions of those output. If anything still uses the
old format, we would not be able to verify their posts anyway.
2024-03-25 14:05:05 -01:00
Oneric d6d838cbe8 StealEmoji: check remote size before downloading
To save on bandwith and avoid OOMs with large files.
Ofc, this relies on the remote server
 (a) sending a content-length header and
 (b) being honest about the size.

Common fedi servers seem to provide the header and (b) at least raises
the required privilege of an malicious actor to a server infrastructure
admin of an explicitly allowed host.

A more complete defense which still works when faced with
a malicious server requires changes in upstream Finch;
see https://github.com/sneako/finch/issues/224
2024-03-18 22:33:10 -01:00
Oneric a4fa2ec9af StealEmoji: make final paths infeasible to predict
Certain attacks rely on predictable paths for their payloads.
If we weren’t so overly lax in our (id, URL) check, the current
counterfeit activity exploit would be one of those.
It seems plausible for future attacks to hinge on
or being made easier by predictable paths too.

In general, letting remote actors place arbitrary data at
a path within our domain of their choosing (sans prefix)
just doesn’t seem like a good idea.

Using fully random filenames would have worked as well, but this
is less friendly for admins checking emoji dirs.
The generated suffix should still be more than enough;
an attacker needs on average 140 trillion attempts to
correctly guess the final path.
2024-03-18 22:33:10 -01:00
Oneric d1c4d07404 Convert StealEmoji to pack.json
This will decouple filenames from shortcodes and
allow more image formats to work instead of only
those included in the auto-load glob. (Albeit we
still saved other formats to disk, wasting space)

Furthermore, this will allow us to make
final URL paths infeasible to predict.
2024-03-18 22:33:10 -01:00
Oneric 5b126567bb StealEmoji: drop superfluous basename
Since 3 commits ago we restrict shortcodes to a subset of
the POSIX Portable Filename Character Set, therefore
this can never have a directory component.
2024-03-18 22:33:10 -01:00