You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@usergrid.apache.org by arun-ram <gi...@git.apache.org> on 2015/07/07 17:21:48 UTC
[GitHub] incubator-usergrid pull request: Ensure that the input application...
GitHub user arun-ram opened a pull request:
https://github.com/apache/incubator-usergrid/pull/300
Ensure that the input application name does not contain characters th…
…at can be used to inject malicious content.
The application name is repeated as-is in the response message if the oragnization/application cannot be found when OrganizationApplicationNotFoundException is thrown. This could be used for an XSS attack.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/arun-ram/incubator-usergrid master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-usergrid/pull/300.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #300
----
commit e4c4dbd66ebb4f4fb61d40a302a7ce200943c1ad
Author: arun.ram <ar...@spartasystems.com>
Date: 2015-07-07T15:17:30Z
Ensure that the input application name does not contain characters that can be used to inject malicious content.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-124862580
I've merged this, if there's more content to add please raise more PRs. The more the merrier.
Thanks for the contrib @arun-ram !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by rgex <gi...@git.apache.org>.
Github user rgex commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-119587099
From RFC 3986:
2.3. Unreserved Characters
Characters that are allowed in a URI but do not have a reserved
purpose are called unreserved. These include uppercase and lowercase
letters, decimal digits, hyphen, period, underscore, and tilde.
unreserved = ALPHA / DIGIT / "-" / "." / "_" / "~"
So an organisation/application/collection should only contain this characters.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:
https://github.com/apache/incubator-usergrid/pull/300#discussion_r34112375
--- Diff: stack/rest/src/main/java/org/apache/usergrid/rest/organizations/OrganizationResource.java ---
@@ -143,21 +143,21 @@ public ApplicationResource getApplicationByName( @PathParam("applicationName") S
}
- @Path("applications/{applicationName}")
+ @Path("applications/{applicationName: [^<>/]+}")
--- End diff --
I tested this out locally, not sure if it's doing what you'd expect. I went to this URL (my local image shows 8580 as the HTTP port)
http://localhost:8580/applications/%3Cscript%3Ealert('bob')%3C/script%3E
The response still included the script tag, so this came back
```
{"error":"organization_application_not_found","timestamp":1436323964157,"duration":3,"exception":"org.apache.usergrid.rest.exceptions.OrganizationApplicationNotFoundException","error_description":"Could not find application for applications/<script>alert('bob')< from URI: applications/<script>alert('bob')</script>"}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by rgex <gi...@git.apache.org>.
Github user rgex commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-121155740
I would suggest that we check the organization, application, collection and connection names.
Similar to CouchDB I think that we should allow ALPHA / DIGIT / "-" / "." / "_" / "~" with one exception: names starting with underscore would be system reserved.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by arun-ram <gi...@git.apache.org>.
Github user arun-ram commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-119263574
I used an exclusion list rather than a white list to allow for non-English characters in the application name. If other languages are not supported, we could change it to a white list.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by rgex <gi...@git.apache.org>.
Github user rgex commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-119262182
Why not use a whitelist such as [-_A-Za-z0-9] ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by arun-ram <gi...@git.apache.org>.
Github user arun-ram commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-122919661
I see that Lynch Lee is working on USERGRID-868 and has a pull request open for validating entity names. @rgex Should we merge this pull request and let Lynch Lee take care of collection and connection names?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:
https://github.com/apache/incubator-usergrid/pull/300#discussion_r34325034
--- Diff: stack/rest/src/test/java/org/apache/usergrid/rest/management/organizations/OrganizationResourceIT.java ---
@@ -32,7 +35,9 @@
import org.apache.usergrid.rest.TestContextSetup;
import junit.framework.Assert;
+import org.usergrid.java.client.response.ApiResponse;
+import static junit.framework.Assert.fail;
--- End diff --
Could you use org.junit here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by johnament <gi...@git.apache.org>.
Github user johnament commented on a diff in the pull request:
https://github.com/apache/incubator-usergrid/pull/300#discussion_r34138213
--- Diff: stack/rest/src/main/java/org/apache/usergrid/rest/organizations/OrganizationResource.java ---
@@ -143,21 +143,21 @@ public ApplicationResource getApplicationByName( @PathParam("applicationName") S
}
- @Path("applications/{applicationName}")
+ @Path("applications/{applicationName: [^<>/]+}")
--- End diff --
If you want to understand why the output always includes the unescaped URI, take a look at [OrganizationApplicationNotFoundException.java](https://github.com/apache/incubator-usergrid/blob/master/stack/rest/src/main/java/org/apache/usergrid/rest/exceptions/OrganizationApplicationNotFoundException.java#L38)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-usergrid/pull/300
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by rgex <gi...@git.apache.org>.
Github user rgex commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-119511047
Collections are also affected
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by arun-ram <gi...@git.apache.org>.
Github user arun-ram commented on a diff in the pull request:
https://github.com/apache/incubator-usergrid/pull/300#discussion_r34372657
--- Diff: stack/rest/src/test/java/org/apache/usergrid/rest/management/organizations/OrganizationResourceIT.java ---
@@ -32,7 +35,9 @@
import org.apache.usergrid.rest.TestContextSetup;
import junit.framework.Assert;
+import org.usergrid.java.client.response.ApiResponse;
+import static junit.framework.Assert.fail;
--- End diff --
done
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-usergrid pull request: Ensure that the input application...
Posted by arun-ram <gi...@git.apache.org>.
Github user arun-ram commented on the pull request:
https://github.com/apache/incubator-usergrid/pull/300#issuecomment-120152413
Based on feedback from rgex, I am now using a whitelist that includes the unreserved characters. I found that I cannot use JAX-RS path param regexp URI matching because the matching is done before URI decoding.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---