Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve federated objects from other instances via redirect (fixes #3129) #4073

Merged
merged 5 commits into from
Oct 25, 2023

Conversation

Nutomic
Copy link
Member

@Nutomic Nutomic commented Oct 20, 2023

Works in testing. Depends on LemmyNet/activitypub-federation-rust#76 but that could possibly introduce security risks.

@Nutomic
Copy link
Member Author

Nutomic commented Oct 24, 2023

Okay this is good to merge, I cant think of any concrete way this could be used for an attack, and its a major usability improvement. Also added an api test.

@Nutomic Nutomic marked this pull request as ready for review October 24, 2023 10:01
Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if I resolved the correct version of your crate, if it passes then I spose we're good.

@dessalines dessalines enabled auto-merge (squash) October 24, 2023 20:33
@Nutomic Nutomic force-pushed the resolve-nonlocal-objects branch from 59b724e to 5972343 Compare October 25, 2023 10:21
@phiresky
Copy link
Collaborator

phiresky commented Oct 25, 2023

Here's some articles about potential issues with open/user-editable redirects :

http://projects.webappsec.org/w/page/13246981/URL%20Redirector%20Abuse

https://developers.google.com/search/blog/2009/01/open-redirect-urls-is-your-site-being

tl;dr: it can make phishing easier because people think they are still on a trusted site but in reality were redirected to a phishing / scam site

@Nutomic
Copy link
Member Author

Nutomic commented Oct 25, 2023

This case is a bit different because the redirects are only served for fetching Activitypub objects with Accept: application/activity+json header. I dont think a browser will ever hit that endpoint for a normal <a href>.

Looks like I'll need to add a lemmy_utils+chrono dependency to lemmy_api_common with default feature set. If restructuring the files would be better I'd appreciate pointers.

Not sure what this is about or how it relates to the PR.

@dessalines dessalines merged commit 568233b into main Oct 25, 2023
@phiresky
Copy link
Collaborator

Sorry, I commented on the wrong PR :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants