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

[3] Ensure that url alias segments correspond to real aliases of articles/categories - with redirect option. #32887

Closed
wants to merge 4 commits into from

Conversation

PhilETaylor
Copy link
Contributor

@PhilETaylor PhilETaylor commented Mar 27, 2021

Issue #32490 and many others

Replacement for #32879 as people wanted it implemented in a b/c way

Implements a redirect or 404 option (and Off by default) to satisfy #32880 (comment)

Take it or leave it. This is the third attempt to fix this perceived security issue.

Summary of Changes

This PR attempts to close a long standing issue where you can manipulate Joomla's urls and the router will still work.

This can be easily seen on the official Joomla.org site at:

https://www.joomla.org/announcements/BLAHBLAHBLAHBLAHBLAHBLAH/5834-LALALALALALALLALALALALALA

https://www.joomla.org/announcements/i/HACK/HACKED/YOUR/SITE/YOU/SUCK/5834-LALALALALALALLALALALALALA

Where BLAHBLAHBLAHBLAHBLAHBLAH is a made up category name and LALALALALALALLALALALALALA is a made up article alias.

That fake url still loads the "Joomla 3.9.25 Release" article which has id 5834. It should, IMHO, and in the opinion of others, be a 404, as LALALALALALALLALALALALALA could be Joomla-Sucks or anything else SEO that a hacker wants to use.

Testing Instructions

Enabled SEF and rewrite. (Legacy, not modern)

Create yourself a category tree of lets say three categories (as a real test, you choose how many you want, it should still work), each a child of the last, and then an article (with alias my-article) in the bottom most category

Top Most -> Middle Most -> Bottom Most -> my-article

Visit the home page and you should see the link to My Article in the Latest Articles Module, click it and get a SEF url of

https://example.com/9-top-most/middle-most/bottom-most/3-my-article

Where 9 is the id of my category "Bottom Most" and 3 is the id of My Article (Yours might be different ids)

Actual result BEFORE applying this Pull Request

Before this PR you can manipulate the url and replace one or more of aliases with HHHH like below, these urls STILL WORK:

https://example.com/9-top-most/middle-most/bottom-most/3-HAHA
https://example.com/9-top-most/middle-most/HAHA/3-HAHA
https://example.com/9-top-most/HAHA/HAHA/3-HAHA
https://example.com/9-HAHA/HAHA/HAHA/3-HAHA

Expected result AFTER applying this Pull Request

After this P, when you manipulate the url and replace one or more of aliases with HHHH like below, these urls DONT WORK: and now lead correctly to a 404 page OR THEY REDIRECT depending on your choice in Articles Config

Screenshot 2021-03-27 at 15 41 42

https://example.com/9-top-most/middle-most/bottom-most/3-HAHA
https://example.com/9-top-most/middle-most/HAHA/3-HAHA
https://example.com/9-top-most/HAHA/HAHA/3-HAHA
https://example.com/9-HAHA/HAHA/HAHA/3-HAHA

But accessing the full generated url with aliases correct STILL WORKS

https://example.com/9-top-most/middle-most/bottom-most/3-my-article

Documentation Changes Required

This is backward compatible - its off by default.

All the new code is doing is running 2n more queries to validate the aliases provided against either the alias of a category or article with a known id, or in the case of nested categories, that ANY category in the db has the provided alias (nor perfect, I know, but much better than it currently is, and this is only for the Middle Most and Top Most category if there are 3 or more nested categories).

// cc @brianteeman @Ruud68 @Bakual @Hackwar

Phil E. Taylor added 2 commits March 26, 2021 17:26
…/categories

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
…rk out the real url

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators PR-staging labels Mar 27, 2021
Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@brianteeman
Copy link
Contributor

What happens with exiting redirects?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32887.

@PhilETaylor

This comment was marked as abuse.

@Bakual
Copy link
Contributor

Bakual commented Mar 27, 2021

I actually love this approach. Especially the redirect option - thanks!

One thing to think about: It may be worth using a variable for the result of $params->get('validateslugs', 0) and use the variable then for the various places where you check the parameter.

Signed-off-by: Phil E. Taylor <phil@phil-taylor.com>
@PhilETaylor

This comment was marked as abuse.

@alikon
Copy link
Contributor

alikon commented Mar 28, 2021

I have tested this item ✅ successfully on d040981


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/32887.

@PhilETaylor

This comment was marked as abuse.

@HLeithner
Copy link
Member

Thanks @PhilETaylor but I don't see this in j3 and as alreadys said no new features for j3

@HLeithner HLeithner closed this Mar 28, 2021
@PhilETaylor

This comment was marked as abuse.

@PhilETaylor PhilETaylor deleted the tighteupurls-withredirect branch March 28, 2021 15:02
@b2z
Copy link
Member

b2z commented Apr 2, 2021

Is it relevant for J4?

@PhilETaylor

This comment was marked as abuse.

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

Successfully merging this pull request may close these issues.

7 participants