dev@glassfish.java.net

Re: REST API and slashes in resource names

From: Oleksiy Stashok <Oleksiy.Stashok_at_Sun.COM>
Date: Thu, 03 Jun 2010 11:01:49 +0200

> QL is not good enough. You may like to run web devtest in this case.
>
> We may like to consider whether we should UDecoder initialized
> depending on the connector
> rather than it is fixed ("true") all the time.
Agree. Because if we will just apply

>> - private final UDecoder urlDecoder = new UDecoder();
>> + private final UDecoder urlDecoder = new UDecoder(true);

we will enable slashesEncoding for *all* listeners, not just admin.
I like the idea to introduce "allowEncodedSlash" (or to be consistent
"encodedSlashEnabled") attribute per network-listener, or better per
<http> protocol.
Here is the snippet how network config looks like [1], where we have
<http> protocol description for each network listener. We can
introduce additional <http> element's attribute "allowEncodedSlash",
which will be false by default. This way we will have
allowEncodedSlash parameter per listener.

Finally <http> protocol description for the admin listener will look
like:

           <protocol name="admin-listener">
             <http default-virtual-server="__asadmin" max-
connections="250" allowEncodedSlash="true">
               <file-cache />
             </http>
           </protocol>

WBR,
Alexey.

[1]
       <network-config>
         <protocols>
           <protocol name="http-listener-1">
             <http default-virtual-server="server">
               <file-cache />
             </http>
           </protocol>
           <protocol security-enabled="true" name="http-listener-2">
             <http default-virtual-server="server">
               <file-cache />
             </http>
             <ssl ssl3-enabled="false" cert-nickname="s1as" />
           </protocol>
           <protocol name="admin-listener">
             <http default-virtual-server="__asadmin" max-
connections="250">
               <file-cache />
             </http>
           </protocol>
         </protocols>
         <network-listeners>
           <network-listener port="${HTTP_LISTENER_PORT}"
protocol="http-listener-1" transport="tcp" name="http-listener-1"
thread-pool="http-thread-pool" />
           <network-listener port="${HTTP_SSL_LISTENER_PORT}"
protocol="http-listener-2" transport="tcp" name="http-listener-2"
thread-pool="http-thread-pool" />
           <network-listener port="${ASADMIN_LISTENER_PORT}"
protocol="admin-listener" transport="tcp" name="admin-listener" thread-
pool="http-thread-pool" />
         </network-listeners>



>
> Shing Wai Chan
>
> On 6/2/10 12:43 PM, Jason Lee wrote:
>>
>> On 6/2/10 11:59 AM, Abhijit Kumar wrote:
>>>
>>> I have to jump in here. The vulnerability is around being able to
>>> traverse outside of the so called "document root" of the server.
>>> Using an encoded URL to get to a file under "document root" is not
>>> a vulnerability.
>>>
>>> IMHO, the answer is fairly simple (and sorry if this was already
>>> discussed). Grizzly should implement a feature to allow
>>> configuration of this feature per connector. Admin connector (port
>>> 4848) should be configured to allow %2F and all the applications
>>> running there should make sure that combination of encoded ".."
>>> and "/" can not be used to get to unauthorized areas of the server.
>> I have a status report on two approaches. The first approach --
>> disable decoding in ContainerMapper and pushing it to the Adapter
>> -- works, it seems. It does, at least for the RestAdapter, and
>> QuickLook didn't seem to be bothered by it. I'm a little worried,
>> though, that it might break something else that's not covered by
>> the QL tests.
>>
>> The second approach, allowing encoded slashes for EVERY Adapter on
>> 4848, also seems to work. The diff for that is pretty simple:
>>
>> Index: kernel/src/main/java/com/sun/enterprise/v3/services/impl/
>> ContainerMapper.java
>> ===================================================================
>> --- kernel/src/main/java/com/sun/enterprise/v3/services/impl/
>> ContainerMapper.java (revision 37388)
>> +++ kernel/src/main/java/com/sun/enterprise/v3/services/impl/
>> ContainerMapper.java (working copy)
>> @@ -81,7 +81,7 @@
>> private Mapper mapper;
>> private GrizzlyEmbeddedHttp grizzlyEmbeddedHttp;
>> private String defaultHostName = "server";
>> - private final UDecoder urlDecoder = new UDecoder();
>> + private final UDecoder urlDecoder = new UDecoder(true);
>> private final Habitat habitat;
>> private final GrizzlyService grizzlyService;
>> protected final static int MAPPING_DATA = 12;
>>
>> Once I updated the jar in my GF instance, URLs like this now work:
>>
>> http://localhost:4848/management/domain/resources/admin-object-resource/jndi%2Ffoo
>>
>> URLs like this, though, do not:
>>
>> http://localhost:4848/management%2F..%2Fmonitoring/domain/server
>> http://localhost:4848/management/..%2Fmonitoring/domain/server
>> http://localhost:4848/management%2F../monitoring/domain/server
>>
>> For the record, this does:
>>
>> http://localhost:4848/management/../monitoring/domain/server
>>
>> I'm not sure if that's a problem or not.
>>
>> Another approach would be to get the Adapter before decoding,
>> create a new UDecoder on every request (new
>> UDecoder(adapter.isAllowEncodedSlash()), then decode the URL.
>> While that shrinks any potential attack surface, it may also run
>> afoul of the possibility of the multibyte characters Shing Wai
>> mentioned, which may or may not be an issue with the GF
>> administration port.
>>
>> Now, having said all of that, we really need to make a decision so
>> this can move on. Aesthetically, single encoding looks better, and
>> it's certainly more intuitive to the end user. In case this hasn't
>> been clear, though, I'm a bit nervous making changes like this due
>> to unintended consequences. I've tried to exploit this change to
>> get to things I'm not supposed to, but haven't been able to yet,
>> for what it's worth.
>>
>> The double encoding approach, while slightly uglier, also works and
>> seems to be free of any security concerns. I also have it
>> implemented already. :P
>>
>> I don't have a strong preference either way, so I'm perfectly
>> content with adopting whatever the general consensus here is, which
>> I'd like to determine as quickly as we can, as we've spent a lot of
>> time on this already, and this issue is holding up MS2 deliverables.
>>
>> Let the voting commence. :)
>> --
>> Jason Lee
>> Senior Member of Technical Staff
>> GlassFish Administration Console
>>
>> Oracle Corporation
>> Phone +1 405-216-3193
>> Blog http://blogs.steeplesoft.com
>