Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

2018-09-08 Thread Wang Weijun
Thinking about this again. Looks like the absolute path is not necessary. Even 
if there are multiple files using the same name, they will be in different 
directories, no matter absolute or relative. Suppose the jarPath info is used 
for debugging purpose mostly like the developer can find out what the current 
working directory is and can find the files. *Matthias*: Is the absolute path 
really necessary? Are you working on actual case?

As for the possible global effect of a security property, maybe we can emphasis 
that this is both a security property and system property, and if it’s just for 
one time use, it’s better to use a system property. 

BTW, does the existing value “hostInfo” of the property have a similar problem?

Thanks
Max

>> 在 2018年9月8日,21:42,Sean Mullan  写道:
>> 
>> On 9/7/18 7:58 PM, Weijun Wang wrote:
>> In my understanding, the author deliberately wants to show the absolute 
>> paths when there are multiple jar files with the same name (Ex: a jar hell).
> 
> If you are very familiar with a particular application and understand the 
> risks associated with running it, then I agree that is ok. But security 
> properties apply to all applications using that JDK, and so I don't always 
> think it is practical for an admin to understand every type of application 
> that may be using that JDK and whether or not enabling this property presents 
> a risk.
> 
>> Maybe we can add some more detail in the java.security so an admin knows 
>> what exact impact it has.
> 
> It would be a slippery slope in my opinion. You would have to say something 
> like: "enabling this property may allow untrusted code running under a 
> SecurityManager to access sensitive information such as the user.home system 
> property even if it has not been granted permission to do so."
> 
> I think we should consider not allowing this property to take effect if a 
> SecurityManager is running.
> 
> --Sean
> 
>> --Max
>>> On Sep 8, 2018, at 3:41 AM, Sean Mullan  wrote:
>>> 
>>> On 8/29/18 10:01 AM, Baesken, Matthias wrote:
 Hi Max, thanks for your input .
 I created another webrev , this contains now   the suggested  
 SecurityProperties   class :
 http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/
>>> 
>>> java/util/jar/Attributes.java
>>> 
>>> 469 return AccessController.doPrivileged(new 
>>> PrivilegedAction() {
>>> 470 public String run() {
>>> 471 return file.getAbsolutePath() + ":" + lineNumber;
>>> 472 }
>>> 473 });
>>> 
>>> I have a serious concern with the code above.
>>> 
>>> With this change, untrusted code running under a SecurityManager could 
>>> potentially create a JarFile on a non-absolute path ex: "foo.jar", and 
>>> (somehow) cause an IOException to be thrown which would then reveal the 
>>> absolute path of where the jar was located, and thus could reveal sensitive 
>>> details about the system (ex: the user's home directory). It could still be 
>>> hard to exploit, since it would have to be a known jar that exists, and you 
>>> would then need to cause an IOException to be thrown, but it's still 
>>> theoretically possible.
>>> 
>>> In short, this is a more general issue in that it may allow untrusted code 
>>> to access something it couldn't have previously. new 
>>> JarFile("foo.jar").getName() returns "foo.jar", and not the absolute path.
>>> 
>>> Granted this can only be done if the security property is enabled, but I 
>>> believe this is not something administrators should have to know about 
>>> their environment and account for when enabling this property.
>>> 
>>> The above code should be changed to return only what the caller provides to 
>>> JarFile, which is the name of the file (without the full path).
>>> 
>>> --Sean



Re: [RFR] 8205525 : Improve exception messages during manifest parsing of jar archives

2018-09-08 Thread Sean Mullan

On 9/7/18 7:58 PM, Weijun Wang wrote:
In my understanding, the author deliberately wants to show the absolute paths when there are multiple jar files with the same name (Ex: a jar hell). 


If you are very familiar with a particular application and understand 
the risks associated with running it, then I agree that is ok. But 
security properties apply to all applications using that JDK, and so I 
don't always think it is practical for an admin to understand every type 
of application that may be using that JDK and whether or not enabling 
this property presents a risk.



Maybe we can add some more detail in the java.security so an admin knows what 
exact impact it has.


It would be a slippery slope in my opinion. You would have to say 
something like: "enabling this property may allow untrusted code running 
under a SecurityManager to access sensitive information such as the 
user.home system property even if it has not been granted permission to 
do so."


I think we should consider not allowing this property to take effect if 
a SecurityManager is running.


--Sean



--Max


On Sep 8, 2018, at 3:41 AM, Sean Mullan  wrote:

On 8/29/18 10:01 AM, Baesken, Matthias wrote:

Hi Max, thanks for your input .
I created another webrev , this contains now   the suggested  
SecurityProperties   class :
http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.6/


java/util/jar/Attributes.java

469 return AccessController.doPrivileged(new PrivilegedAction() 
{
470 public String run() {
471 return file.getAbsolutePath() + ":" + lineNumber;
472 }
473 });

I have a serious concern with the code above.

With this change, untrusted code running under a SecurityManager could potentially create 
a JarFile on a non-absolute path ex: "foo.jar", and (somehow) cause an 
IOException to be thrown which would then reveal the absolute path of where the jar was 
located, and thus could reveal sensitive details about the system (ex: the user's home 
directory). It could still be hard to exploit, since it would have to be a known jar that 
exists, and you would then need to cause an IOException to be thrown, but it's still 
theoretically possible.

In short, this is a more general issue in that it may allow untrusted code to access something it 
couldn't have previously. new JarFile("foo.jar").getName() returns "foo.jar", 
and not the absolute path.

Granted this can only be done if the security property is enabled, but I 
believe this is not something administrators should have to know about their 
environment and account for when enabling this property.

The above code should be changed to return only what the caller provides to 
JarFile, which is the name of the file (without the full path).

--Sean