Hi Matthias, I think it would be enough to drop the privileged section and just return "filename" as is. (without conveting to a file object).
I also agree with Sean that the common parts of the new src/java.base/share/classes/sun/security/util/SecurityProperties.java should be aligned with src/java.base/share/classes/sun/net/util/SocketExceptions.java (That is, SocketExceptions calling SecurityProperties) Best regards Christoph > -----Original Message----- > From: core-libs-dev <core-libs-dev-boun...@openjdk.java.net> On Behalf > Of Baesken, Matthias > Sent: Montag, 10. September 2018 09:53 > To: Wang Weijun <weijun.w...@oracle.com>; Sean Mullan > <sean.mul...@oracle.com> > Cc: security-dev@openjdk.java.net; core-libs-...@openjdk.java.net > Subject: [CAUTION] RE: [RFR] 8205525 : Improve exception messages during > manifest parsing of jar archives > > Hello are you fine with changing from file.getAbsolutePath() to > file.getName() ? > > > http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.8/src/java.base/s > hare/classes/java/util/jar/Manifest.java.frames.html > > > 206 static String getErrorPosition(String filename, final int lineNumber) > { > 207 if (filename == null || !jarPathInExceptionText) { > 208 return "line " + lineNumber; > 209 } > 210 > 211 final File file = new File(filename); > 212 return AccessController.doPrivileged(new > PrivilegedAction<String>() > { > 213 public String run() { > 214 return "manifest of " + file.getName() + ":" + > lineNumber; > 215 } > > > Best regards, Matthias > > > > -----Original Message----- > > From: Wang Weijun <weijun.w...@oracle.com> > > Sent: Samstag, 8. September 2018 17:43 > > To: Sean Mullan <sean.mul...@oracle.com> > > Cc: Baesken, Matthias <matthias.baes...@sap.com>; Alan Bateman > > <alan.bate...@oracle.com>; Chris Hegarty <chris.hega...@oracle.com>; > > security-dev@openjdk.java.net; core-libs-...@openjdk.java.net > > Subject: Re: [RFR] 8205525 : Improve exception messages during manifest > > parsing of jar archives > > > > 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 <sean.mul...@oracle.com> 写 > > 道： > > >> > > >> 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 <sean.mul...@oracle.com> > > 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<String>() { > > >>> 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
