syncope-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Francesco Chicchiriccò <ilgro...@apache.org>
Subject Re: Some queries on getMetadata in SAML2SPLogic
Date Mon, 14 Aug 2017 15:13:58 GMT
Hi,
I have committed the changes discussed below as

https://github.com/apache/syncope/commit/e99766a441260bb47dbac13efe069682dd4a442d
https://github.com/apache/syncope/commit/f912d90c2aa23c055cfb2e143e865f02025154bd

Regards.

On 11/08/2017 18:02, Colm O hEigeartaigh wrote:
> On Fri, Aug 11, 2017 at 2:59 PM, Francesco Chicchiriccò <ilgrosso@apache.org>
wrote:
>
>> Agree. Maybe it should just be changed to
>>
>> @PreAuthorize("isAuthenticated()")
> +1.
>
>
>> b) The urlContext not validated at all. For example, you can pass through
>>> something like  "../../root" which is added to the metadata, e.g.
>>> Location="
>>> http://localhost:9080/syncope/../../root/assertion-consumer".
>>>
>>> Should we implement some kind of validation rules on what is acceptable
>>> here?
>>>
>> What do you have in mind here? Just forbid '../'? What could be the
>> issue(s) with the current implementation?
>
> Well it makes me a bit uneasy that  a user can essentially insert arbitrary
> URLs into a metadata document issued by Syncope. I have three suggestions:
>
> a) Restrict the acceptable values. Do we need to allow arbitrary contexts
> here...?
> b) Disallow ".." in the values
> c) Use Commons UrlValidator to make sure that the generated assertion
> consumer URL is a valid URL.
>
> Colm.

-- 
Francesco Chicchiriccò

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/


Mime
View raw message