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

URI Restrictions ~ Make CaSe InsensitiVe #354

Closed
anatomism opened this issue Oct 5, 2014 · 6 comments
Closed

URI Restrictions ~ Make CaSe InsensitiVe #354

anatomism opened this issue Oct 5, 2014 · 6 comments

Comments

@anatomism
Copy link

I've been using S2 member for nearly a year. I use it on a multi site network, whereby one of the installs is entirely protected by S2 member "/staff".

Under the URI restrictions, this was set to "staff/" and it worked well, protecting "www.domain.com/staff", however as discovered yesterday, the URI restriction is case sensitive and navigating to "www.domain.com/STAFF" would bypass the URI restriction and give full access to anyone. I could not find anything in the documentation about this. My (hopefully temporary) solution was to list all 32 iterations of 'staff' with mixed case. If this is usual behaviour, I suggest warning users. In the next update

Thanks

@jaswrks jaswrks added this to the Next Release milestone Oct 5, 2014
@jaswrks
Copy link
Contributor

jaswrks commented Oct 6, 2014

@anatomism Thanks for this report!

@clavaque Do you see any problems if we make URI Restrictions caSe insensitive?

@jaswrks jaswrks self-assigned this Oct 6, 2014
@jaswrks
Copy link
Contributor

jaswrks commented Oct 6, 2014

I think the issue here is partly related to some changes in WP. At one time, URIs were canonicalized automatically, to always use the caSe provided by a site owner. That seems to not be the case any longer, leaving room for this to be exploited in order to gain access to protected areas.

@clavaque
Copy link
Contributor

clavaque commented Oct 6, 2014

Well, normally URIs are case sensitive, but if WordPress will now ignore casing for slugs, I think that we should follow suit. It's no good that we follow the web's standard regarding that, if WP will ignore it and let anyone sidestep the restriction. I'm sure there will be no-one that wants "staff" to be restricted but leave "Staff" public.

@jaswrks
Copy link
Contributor

jaswrks commented Oct 6, 2014

Agreed! The only problem that comes to mind, is related to query string arguments, where caSe might matter to some of the existing installations out there. I don't see that as a major issue though, and in fact it seems more user-friendly if this is not caSe sensitive in any way.

@jaswrks jaswrks changed the title URI Restrictions is case sensitive URI Restrictions ~ Make CaSe InsensitiVe Oct 6, 2014
@jaswrks jaswrks mentioned this issue Oct 8, 2014
@jaswrks
Copy link
Contributor

jaswrks commented Oct 8, 2014

Next release changelog:

  • (s2Member/s2Member Pro) URI Restrictions caSe-insensitive (Security Fix) This release of s2Member changes the way URI Restrictions work. All URI Restrictions are now caSe-insensitive (i.e. /some-path/ is now the same as /some-Path/), allowing s2Member to automatically pick up different variations used in attempts to exploit the behavior of certain slugs within the WordPress core. You can also change this new default behavior, if you prefer. Please see: Dashboard ⥱ s2Member ⥱ Restriction Options ⥱ URI Restrictions. See also: this GitHub issue for the details about why this was changed in the most recent copy of s2Member.

@jaswrks jaswrks closed this as completed Oct 8, 2014
@jaswrks
Copy link
Contributor

jaswrks commented Oct 8, 2014

2014-10-07_17-49-00

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

No branches or pull requests

3 participants