Grails

UrlMappingsFilter bug with action ending in '.'

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Critical Critical
  • Resolution: Fixed
  • Affects Version/s: 1.0.4
  • Fix Version/s: 1.0.5, 1.1-beta3
  • Component/s: Controllers
  • Labels:
    None
  • Patch Submitted:
    Yes

Description

If someone requests a URL that ends in '.' but is otherwise a valid controller action (e.g. "/admin/user/list.") the 'list' action is displayed when it should have been a 404. This is caused by the call to WebUtils.getFormatFromURI(uri) in UrlMappingsFilter (line 88 in 1.0.4, line 101 in the 1.1 branch).

It checks for a null format and since a blank string is returned in this case, it's tricked into thinking it's a file-extension request and trims off the dot. If you change "if(format!=null)" to "if(StringUtils.hasLength(format))" then it works as it should.

This is slightly more urgent than it appears since this creates a back door if using the Acegi plugin; "/admin/user/list." doesn't match the typical security mapping of "/admin/user/list/*=ROLE_WHATEVER". The workaround is to add an additional mapping for "/admin/user/list=ROLE_WHATEVER" but it's not very DRY.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Burt Beckwith added a comment -

Jira ate my asterisks - the two mappings should be

/admin/user/list/@@=ROLE_WHATEVER

and

/admin/user/list@=ROLE_WHATEVER

replace @ with an asterisk.

Show
Burt Beckwith added a comment - Jira ate my asterisks - the two mappings should be /admin/user/list/@@=ROLE_WHATEVER and /admin/user/list@=ROLE_WHATEVER replace @ with an asterisk.
Hide
Permalink
Peter Ledbrook added a comment -

Up-rating this issue - not good to have easy ways of bypassing security checks!

Show
Peter Ledbrook added a comment - Up-rating this issue - not good to have easy ways of bypassing security checks!
Hide
Permalink
Peter Ledbrook added a comment -

Hi Burt,

Not quite sure why "/admin/user/list.xml" isn't also a problem. What's the difference?

Thanks,

Peter

Show
Peter Ledbrook added a comment - Hi Burt, Not quite sure why "/admin/user/list.xml" isn't also a problem. What's the difference? Thanks, Peter
Hide
Permalink
Burt Beckwith added a comment -

I think this is less of an issue than I thought - it's more of a documentation problem. You wouldn't guard UserController.list() with /user/list/**, you'd naively guard it with /user/list and properly with /user/list*. I was thinking about a bigger problem that I fixed in the 0.5.1 release where URLs containing a dot weren't checked at all, so mapping /user/** wouldn't block access to "/user/list." and this bug would result in "/user/list" being rendered.

It behaves differently in 1.1. It only strips off the extension if it's a supported mime type, so "list.xml" goes through to the "list" action and shows the page, while "list." generates the same 404 that you'd get in 1.0.4 with the proposed fix since getArtefactForFeature() returns null.

Show
Burt Beckwith added a comment - I think this is less of an issue than I thought - it's more of a documentation problem. You wouldn't guard UserController.list() with /user/list/**, you'd naively guard it with /user/list and properly with /user/list*. I was thinking about a bigger problem that I fixed in the 0.5.1 release where URLs containing a dot weren't checked at all, so mapping /user/** wouldn't block access to "/user/list." and this bug would result in "/user/list" being rendered. It behaves differently in 1.1. It only strips off the extension if it's a supported mime type, so "list.xml" goes through to the "list" action and shows the page, while "list." generates the same 404 that you'd get in 1.0.4 with the proposed fix since getArtefactForFeature() returns null.
Hide
Permalink
Peter Ledbrook added a comment -

OK, my plan is to modify WebUtils.getFormatFromURI(uri) so that it returns null if there is no extension after the ".". If that sounds fine to you, I'll commit and resolve the issue.

Show
Peter Ledbrook added a comment - OK, my plan is to modify WebUtils.getFormatFromURI(uri) so that it returns null if there is no extension after the ".". If that sounds fine to you, I'll commit and resolve the issue.
Hide
Permalink
Burt Beckwith added a comment -

Makes sense to me - thanks.

Show
Burt Beckwith added a comment - Makes sense to me - thanks.
Hide
Permalink
Graeme Rocher added a comment -

Bulk closing bunch of resolved issues

Show
Graeme Rocher added a comment - Bulk closing bunch of resolved issues

People

  • Assignee:
    Peter Ledbrook
    Reporter:
    Burt Beckwith
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: