Thanks for looking into this!
I understand your concern about introducing a dependency on SLF4J. In previous forks of jts-core, we used an internal API compatible with Log4J and LogCat.
For this effort, I used SLF4J to avoid requiring others who might want the changes to depend on our API. I’m running out of time on my task but I’ll create a PR with the changes I’ve made should you find them beneficial.
Most of the changes replace System.out/System.err to use SLF4J. It will also contains changes to org.locationtech.jts.io.gml2.GeometryStrategies to address Fortify
vulnerabilities to that class. Fortify also reports a vulnerability in org.locationtech.jtstest.util.CommandRunner as noted below. I did not attempt to address that vulnerability.
Thanks again for your time and use of the work done by this group.
Fortify Findings (after SLF4J changes)
1.
org.locationtech.jts.io.gml2.GeometryStrategies (Denial of Service/Regular Expressions)
a.
decimal attribute
i. 323: getValue(return)
ii. 323: Addignment to decimal
iii. 416: replaceAll(0)
b.
cs attribute
i.
c.
ts attribute
i.
Recommendations:
Do not allow untrusted data to be used as regular _expression_ patterns.
References:
[1] Bryan Sullivan, Regular _expression_ Denial of Service Attacks and Defenses, http://msdn.microsoft.com/en-us/magazine/ff646973.aspx
[2] IDS08-J. Sanitize untrusted data included in a regular _expression_, CERT, https://www.securecoding.cert.org/confluence/display/java/IDS08-J.+Sanitize+untrusted+data+included+in+a+regular+_expression_
[3] DOS-1: Beware of activities that may use disproportionate resources, Oracle, http://www.oracle.com/technetwork/java/seccodeguide-139067.html#1
[4] INPUT-1: Validate inputs, Oracle, http://www.oracle.com/technetwork/java/seccodeguide-139067.html#5
[5] Standards Mapping - Common Weakness Enumeration, CWE ID 185, CWE ID 730
[6] Standards Mapping - NIST Special Publication 800-53 Revision 4, SC-5 Denial of Service Protection (P1)
[7] Standards Mapping - OWASP Top 10 2004, A9 Application Denial of Service
[8] Standards Mapping - Payment Card Industry Data Security Standard Version 1.1, Requirement 6.5.9
[9] Standards Mapping - Payment Card Industry Data Security Standard Version 3.0, Requirement 6.5.6
[10] Standards Mapping - Payment Card Industry Data Security Standard Version 3.1, Requirement 6.5.6
[11] Standards Mapping - Security Technical Implementation Guide Version 3.1, APP6080 CAT II
[12] Standards Mapping - Security Technical Implementation Guide Version 3.10, APP6080 CAT II
[13] Standards Mapping - Security Technical Implementation Guide Version 3.4, APP6080 CAT II
[14] Standards Mapping - Security Technical Implementation Guide Version 3.5, APP6080 CAT II
[15] Standards Mapping - Security Technical Implementation Guide Version 3.6, APP6080 CAT II
[16] Standards Mapping - Security Technical Implementation Guide Version 3.7, APP6080 CAT II
[17] Standards Mapping - Security Technical Implementation Guide Version 3.9, APP6080 CAT II
[18] Standards Mapping - Web Application Security Consortium 24 + 2, Denial of Service
[19] Standards Mapping - Web Application Security Consortium Version 2.00, Denial of Service (WASC-10)
2.
org.locationtech.jtstest.util.CommandRunner (Command Injection)
a.
cmd String
i. CommandPanel 299: getText(return)
ii. CommandPanel 299: Assognment to cmd
iii. CommandPanel 304:execCommand(1)
iv. CommandController 39: expandCommand(0 : return)
v. CommandController 39: assignment to cmd
vi. CommandController 56: exec(0)
vii. CommandRunner 45: cmdArray(0 : return[2])
viii. CommandRunner 45: Assignment to osCmd
ix. CommandRunner 47: exec(0)
Recommendations:
Do not allow users to have direct control over the commands executed by the program. In cases where user input must affect the command
to be run, use the input only to make a selection from a predetermined set of safe commands. If the input appears to be malicious, the value passed to the command execution function should either default to some safe selection from this set or the program
should decline to execute any command at all.
In cases where user input must be used as an argument to a command executed by the program, this approach often becomes impractical because
the set of legitimate argument values is too large or too hard to keep track of. Developers often fall back on blacklisting in these situations. Blacklisting selectively rejects or escapes potentially dangerous characters before using the input. Any list of
unsafe characters is likely to be incomplete and will be heavily dependent on the system where the commands are executed. A better approach is to create a whitelist of characters that are allowed to appear in the input and accept input composed exclusively
of characters in the approved set.
An attacker can indirectly control commands executed by a program by modifying the environment in which they are executed. The environment
should not be trusted and precautions should be taken to prevent an attacker from using some manipulation of the environment to perform an attack. Whenever possible, commands should be controlled by the application and executed using an absolute path. In cases
where the path is not known at compile time, such as for cross-platform applications, an absolute path should be constructed from trusted values during execution. Command values and paths read from configuration files or the environment should be sanity-checked
against a set of invariants that define valid values.
Other checks can sometimes be performed to detect if these sources may have been tampered with. For example, if a configuration file
is world-writable, the program might refuse to run. In cases where information about the binary to be executed is known in advance, the program may perform checks to verify the identity of the binary. If a binary should always be owned by a particular user
or have a particular set of access permissions assigned to it, these properties can be verified programmatically before the binary is executed.
Although it may be impossible to completely protect a program from an imaginative attacker bent on controlling the commands the program
executes, be sure to apply the principle of least privilege wherever the program executes an external command: do not hold privileges that are not essential to the execution of the command.
Tips:
1. A number of modern web frameworks provide mechanisms for performing validation of user input. Struts and Spring MVC are among them.
To highlight the unvalidated sources of input, the HPE Security Fortify Secure Coding Rulepacks dynamically re-prioritize the issues reported by HPE Security Fortify Static Code Analyzer by lowering their probability of exploit and providing pointers to the
supporting evidence whenever the framework validation mechanism is in use. We refer to this feature as Context-Sensitive Ranking. To further assist the HPE Security Fortify user with the auditing process, the HPE Security Fortify Software Security Research
group makes available the Data Validation project template that groups the issues into folders based on the validation mechanism applied to their source of input.
References:
[1] IDS07-J. Sanitize untrusted data passed to the Runtime.exec() method, CERT, https://www.securecoding.cert.org/confluence/display/java/IDS07-J.+Sanitize+untrusted+data+passed+to+the+Runtime.exec%28%29+method
[2] INPUT-1: Validate inputs, Oracle, http://www.oracle.com/technetwork/java/seccodeguide-139067.html#5
[3] Standards Mapping - Common Weakness Enumeration, CWE ID 77, CWE ID 78
[4] Standards Mapping - NIST Special Publication 800-53 Revision 4, SI-10 Information Input Validation (P1)
[5] Standards Mapping - OWASP Mobile Top 10 Risks 2014, M7 Client Side Injection
[6] Standards Mapping - OWASP Top 10 2004, A6 Injection Flaws
[7] Standards Mapping - OWASP Top 10 2007, A2 Injection Flaws
[8] Standards Mapping - OWASP Top 10 2010, A1 Injection
[9] Standards Mapping - OWASP Top 10 2013, A1 Injection
[10] Standards Mapping - Payment Card Industry Data Security Standard Version 1.1, Requirement 6.5.6
[11] Standards Mapping - Payment Card Industry Data Security Standard Version 1.2, Requirement 6.3.1.1, Requirement 6.5.2
[12] Standards Mapping - Payment Card Industry Data Security Standard Version 2.0, Requirement 6.5.1
[13] Standards Mapping - Payment Card Industry Data Security Standard Version 3.0, Requirement 6.5.1
[14] Standards Mapping - Payment Card Industry Data Security Standard Version 3.1, Requirement 6.5.1
[15] Standards Mapping - SANS Top 25 2009, Insecure Interaction - CWE ID 116
[16] Standards Mapping - SANS Top 25 2010, Insecure Interaction - CWE ID 078
[17] Standards Mapping - SANS Top 25 2011, Insecure Interaction - CWE ID 078
[18] Standards Mapping - Security Technical Implementation Guide Version 3.1, APP3510 CAT I, APP3570 CAT I
[19] Standards Mapping - Security Technical Implementation Guide Version 3.10, APP3510 CAT I, APP3570 CAT I
[20] Standards Mapping - Security Technical Implementation Guide Version 3.4, APP3510 CAT I, APP3570 CAT I
[21] Standards Mapping - Security Technical Implementation Guide Version 3.5, APP3510 CAT I, APP3570 CAT I
[22] Standards Mapping - Security Technical Implementation Guide Version 3.6, APP3510 CAT I, APP3570 CAT I
[23] Standards Mapping - Security Technical Implementation Guide Version 3.7, APP3510 CAT I, APP3570 CAT I
[24] Standards Mapping - Security Technical Implementation Guide Version 3.9, APP3510 CAT I, APP3570 CAT I
[25] Standards Mapping - Web Application Security Consortium 24 + 2, OS Commanding
[26] Standards Mapping - Web Application Security Consortium Version 2.00, OS Commanding (WASC-31)
Much obliged,
From: jts-dev <jts-dev-bounces@xxxxxxxxxxx>
On Behalf Of Martin Davis
Sent: Thursday, March 31, 2022 12:47 PM
To: JTS project developer mailing list <jts-dev@xxxxxxxxxxx>
Subject: Re: [jts-dev] [EXTERNAL] Re: Modify JTS Core to use a logging framework
EXTERNAL EMAIL -- This message originates from outside of SAIC
I have had a chance to look at usage of System.out in JTS-core, and I stand corrected: there are over 1K references to it in live code!
This is for a variety of uses. Some are in methods which are only intended to support debugging. Some are in code which should be moved to the test package. And there are probably other cases - haven't looked through them all (for obvious
reasons).
This said, it will require more thought about whether the solution is to replace all usages with output to a logging library. One thing is that if that is done I'd prefer to wrap the logging in an internal API (e.g. perhaps using the Debug
class), to avoid proliferating an external API through the code.
I'd also like to go through and remove or move as many System.out calls as possible. But that's going to be a long-term exercise, obviously.
It sounds like it is in test code, and in that case the JARs should certainly be separated, and only the actual core jars used.
I would suggest separating the submissions - one for functional changes (the GeometryStrategies change) and one or more for the logging/stack dump related changes. I also wonder if the output is limited to test code, and if so if this should simply be separated
into a distinct jar from the operational code.
On 3/30/2022 4:05 PM, Bryant, Phil L. wrote:
While I’m hopeful the following changes can be merged into a future release, I certainly understand
if it goes against the JTS design and the PR is rejected.
The information contained in this e-mail and any attachments from Science Applications International Corporation ("SAIC") may contain confidential and/or proprietary information, and is intended only for the named recipient to whom it was originally addressed.
If you are not the intended recipient, any disclosure, distribution, or copying of this e-mail or its attachments is strictly prohibited. If you have received this e-mail in error, please notify the sender immediately by return e-mail and permanently delete
the e-mail and any attachments.
