[
https://issues.apache.org/jira/browse/IO-712?focusedWorklogId=544579&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-544579
]
ASF GitHub Bot logged work on IO-712:
-------------------------------------
Author: ASF GitHub Bot
Created on: 29/Jan/21 23:48
Start Date: 29/Jan/21 23:48
Worklog Time Spent: 10m
Work Description: garydgregory commented on a change in pull request #197:
URL: https://github.com/apache/commons-io/pull/197#discussion_r567153417
##########
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:
This is a special case. The whole purpose of this API is to never blow
up. I am not sure how useful this method really is in practice though. If you
are running under JAAS and this call returns null because you can't read a sys
prop and the rest of your app runs (or not), then you must have a very specific
set of JAAS permissions set. But... I can see that one might want to write out
or log sys props on start up, and then, writing 'null' instead of blowing up
provides... what? I'm not sure. All of this rambling to say that in this case
we should not change the behavior IMO.
Writing to the console is a no-no so we should get rid of that for sure.
----------------------------------------------------------------
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: 544579)
Time Spent: 0.5h (was: 20m)
> 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: 0.5h
> 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)
