[Security] StealEmojiPolicy: Sanitize shortcodes #701

Merged

floatingghost merged 2 commits from erincandescent/akkoma:stealemojipolicy-sanitize into develop 2024-02-20 15:08:55 +00:00

Conversation 3 Commits 2 Files changed 2 +28

erincandescent commented 2024-02-20 10:22:46 +00:00

Contributor

Copy link

Pleroma announced this one as a security vuln and our code is identical here. It sems to allow directory traversal, which would allow a stolen emoji to smash over one from a pack.

I wonder if we should also hardcode a list of emoji names which are always banned such as "." and "..". Then again, are dots even allowed in shortcodes? Can we just ban shortcodes with dots?

Pleroma announced this one as a security vuln and our code is identical here. It sems to allow directory traversal, which would allow a stolen emoji to smash over one from a pack. I wonder if we should also hardcode a list of emoji names which are always banned such as `"."` and `".."`. Then again, are dots even allowed in shortcodes? Can we just ban shortcodes with dots?

erincandescent added 1 commit 2024-02-20 10:22:46 +00:00

StealEmojiPolicy: Sanitize shortcodes ... 7d94476dd6

Closes: https://git.pleroma.social/pleroma/pleroma/-/issues/3245

erincandescent added 1 commit 2024-02-20 10:36:21 +00:00

Don't steal emoji who's shortcodes have dots or colons in their name ... b387f4a1c1

Mastodon at the very least seems to prevent the creation of emoji with
dots in their name (and refuses to accept them in federation). It feels
like being cautious in what we accept is reasonable here.

Colons are the emoji separator and so obviously should be blocked.

Perhaps instead of filtering out things like this we should just
do a regex match on `[a-zA-Z0-9_-]`? But that's plausibly a decision
for another day

    Perhaps we should also have a centralised "is this a valid emoji shortcode?"
    function

erincandescent commented 2024-02-20 10:37:07 +00:00

Author

Contributor

Copy link

You know what, I don't see a good reason to allow dots or colons, so we should probably block those too

You know what, I don't see a _good_ reason to allow dots or colons, so we should probably block those too

erincandescent commented 2024-02-20 10:45:16 +00:00

Author

Contributor

Copy link

The fact that this just takes the extension from the origin seems potentially smelly to me (emoji.activitystreams2 anyone?) but at least this leaves things in a safer state than we found it

The fact that this just takes the extension from the origin seems potentially smelly to me (`emoji.activitystreams2` anyone?) but at least this leaves things in a safer state than we found it

floatingghost commented 2024-02-20 15:08:50 +00:00

Owner

Copy link

thanks for pulling this over, i'll format it my side to get this in dev asap

thanks for pulling this over, i'll format it my side to get this in dev asap

floatingghost merged commit 2f9aad0e65 into develop 2024-02-20 15:08:55 +00:00

floatingghost deleted branch stealemojipolicy-sanitize 2024-02-20 15:08:55 +00:00

floatingghost referenced this pull request from a commit 2024-02-20 15:08:57 +00:00

Merge pull request '[Security] StealEmojiPolicy: Sanitize shortcodes' (#701) from erincandescent/akkoma:stealemojipolicy-sanitize into develop

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

approved, awaiting change

  

bug

Something is not working

  

configuration

This requires config changes

  

documentation

This is about human-readable docs

  

duplicate

This issue or pull request already exists

  

enhancement

New feature

  

extremely low priority

  

feature request

Something new?

  

Fix it yourself

Support has been attempted

  

help wanted

Need some help

  

invalid

Something is wrong

  

mastodon_api

  

needs docs

  

needs tests

  

not a bug

  

planned

  

pleroma_api

  

privacy

  

question

More information is needed

  

static_fe

  

triage

  

wontfix

This won't be fixed

No labels

approved, awaiting change

bug

configuration

documentation

duplicate

enhancement

extremely low priority

feature request

Fix it yourself

help wanted

invalid

mastodon_api

needs docs

needs tests

not a bug

planned

pleroma_api

privacy

question

static_fe

triage

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: AkkomaGang/akkoma#701

No description provided.