[
https://issues.apache.org/jira/browse/IO-712?focusedWorklogId=544707&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-544707
]
ASF GitHub Bot logged work on IO-712:
-------------------------------------
Author: ASF GitHub Bot
Created on: 30/Jan/21 08:39
Start Date: 30/Jan/21 08:39
Worklog Time Spent: 10m
Work Description: boris-unckel commented on a change in pull request #197:
URL: https://github.com/apache/commons-io/pull/197#discussion_r567218271
##########
File path: src/main/java/org/apache/commons/io/FileSystem.java
##########
@@ -146,27 +146,16 @@ private static boolean getOsMatchesName(final String
osNamePrefix) {
}
/**
- * <p>
- * Gets a System property, defaulting to {@code null} if the property
cannot be read.
- * </p>
- * <p>
- * If a {@code SecurityException} is caught, the return value is {@code
null} and a message is written to
- * {@code System.err}.
- * </p>
+ * Gets a System property, defaulting to {@code null} if the property
cannot be read,
+ * except for {@code SecurityException}.
*
* @param property
* the system property name
- * @return the system property value or {@code null} if a security problem
occurs
+ * @return the system property value or {@code null} if a problem occurs
+ * @throws SecurityException if underlying calls fail due to missing
permissions.
*/
private static String getSystemProperty(final String property) {
- try {
- return System.getProperty(property);
- } catch (final SecurityException ex) {
- // we are not allowed to look at this property
- System.err.println("Caught a SecurityException reading the system
property '" + property
- + "'; the SystemUtils property value will default to
null.");
- return null;
- }
+ return System.getProperty(property);
Review comment:
I have already commented on the use case, but I'll try again with a
impact analysis of the change:
a) The absolute majority: The Java Security Manager is not active, before
and after change zero impact.
b) The absolute minority: One has the ability to upgrade the library, but
not to change the permissions.xml or xyz.policy file. But the security manager
is in use. This is very unlikely but in this case you're stuck on the old
version. People activating the JSM without ability to grant permissions?
c) The happy case: One has the ability to upgrade the library and to grant
permissions. With the clear message what is is missing one can grant the single
permission to the updated single library.
d) JavaEE users will have no impact with the container itself, permissions
are granted by the container. For their apps, they utilize not single
permissions, but permissions.xml for the whole app. Means the grant is for all
classes in the ear or war, not only for a single jar. In example Spring
requires a *
[PropertyPermission](https://docs.oracle.com/javase/8/docs/technotes/guides/security/permissions.html#PropertyPermission)
calling
[System.getProperties](https://docs.oracle.com/javase/8/docs/api/java/lang/System.html#getProperties--).
If a user does need to read a property without grant there is a way:
[https://docs.oracle.com/javase/8/docs/technotes/guides/security/doprivileged.html](https://docs.oracle.com/javase/8/docs/technotes/guides/security/doprivileged.html)
This way should never be used in a utility method. See chapters "Least
Privilege", "More Privilege", "Reflection".
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 544707)
Time Spent: 50m (was: 40m)
> SecurityExceptions are hidden instead of breaking the regular flow
> ------------------------------------------------------------------
>
> Key: IO-712
> URL: https://issues.apache.org/jira/browse/IO-712
> Project: Commons IO
> Issue Type: Bug
> Components: Utilities
> Affects Versions: 2.8.0
> Reporter: Boris Unckel
> Priority: Critical
> Labels: Security, SecurityManager
> Time Spent: 50m
> Remaining Estimate: 0h
>
> Several points in the code hide SecurityException. These _must_ always
> _break_ the regular control flow, if you're not the SecurityManager.
> UseCase A: One wants to configure the SecurityManager and grant permissions.
> Part of the application is to delete a file. If the permission is missing,
> cleaning does not work. The missing exception does not allow to recognize
> that.
> UseCase B: One has activated the SecurityManager. An attacker abuses the
> relevant method. The missing SecurityException hides this attempt, ones IDS
> can't alarm.
> UseCase C: One utilizes the SecurityManager to test the system, to ensure
> every property (like file location) is set properly. The missing
> SecurityException does not support this UseCase.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
