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

[Baesken, Matthias]

Hello are you fine with  changing from  file.getAbsolutePath()    to   file.getName() ?


http://cr.openjdk.java.net/~mbaesken/webrevs/8205525.8/src/java.base/share/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.wang at oracle.com>
> Sent: Samstag, 8. September 2018 17:43
> To: Sean Mullan <sean.mullan at oracle.com>
> Cc: Baesken, Matthias <matthias.baesken at sap.com>; Alan Bateman
> <Alan.Bateman at oracle.com>; Chris Hegarty <chris.hegarty at oracle.com>;
> security-dev at openjdk.java.net; core-libs-dev at 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.mullan at 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.mullan at 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