Skip to content

Commit 05605c4

Browse files
authored
Merge pull request #1 from kwwall/disabledByDefault
Initial code to disable encodeForSQL
2 parents e232291 + 2474d44 commit 05605c4

File tree

6 files changed

+222
-8
lines changed

6 files changed

+222
-8
lines changed

src/main/java/org/owasp/esapi/ESAPI.java

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,13 @@
1616
*/
1717
package org.owasp.esapi;
1818

19+
import java.util.Arrays;
20+
1921
import javax.servlet.http.HttpServletRequest;
2022
import javax.servlet.http.HttpServletResponse;
2123

2224
import org.owasp.esapi.util.ObjFactory;
25+
import org.owasp.esapi.errors.ConfigurationException;
2326

2427
/**
2528
* ESAPI locator class is provided to make it easy to gain access to the current ESAPI classes in use.
@@ -93,16 +96,18 @@ public static Authenticator authenticator() {
9396
}
9497

9598
/**
96-
* The ESAPI Encoder is primarily used to provide <i>output</i> encoding to
99+
* The ESAPI {@code Encoder} is primarily used to provide <i>output</i> encoding to
97100
* prevent Cross-Site Scripting (XSS).
98-
* @return the current ESAPI Encoder object being used to encode and decode data for this application.
101+
* @return the current ESAPI {@code Encoder} object being used to encode and decode data for this application.
99102
*/
100103
public static Encoder encoder() {
101104
return ObjFactory.make( securityConfiguration().getEncoderImplementation(), "Encoder" );
102105
}
103106

104107
/**
105-
* @return the current ESAPI Encryptor object being used to encrypt and decrypt data for this application.
108+
* ESAPI {@code Encryptor} provides a set of methods for performing common encryption, random number, and
109+
* hashing operations.
110+
* @return the current ESAPI {@code Encryptor} object being used to encrypt and decrypt data for this application.
106111
*/
107112
public static Encryptor encryptor() {
108113
return ObjFactory.make( securityConfiguration().getEncryptionImplementation(), "Encryptor" );
@@ -221,4 +226,74 @@ public static String initialize( String impl ) {
221226
public static void override( SecurityConfiguration config ) {
222227
overrideConfig = config;
223228
}
229+
230+
// KWW - OPEN ISSUE: I don't like placing this here, but it's convenient and I
231+
// don't really know a better place for it and would rather not create
232+
// a whole new utility class just to use it.
233+
/**
234+
* Determine if a given fully qualified (ESAPI) method name has been explicitly
235+
* enabled in the <b>ESAPI.properties</b>'s file via the property name
236+
* <b>ESAPI.dangerouslyAllowUnsafeMethods.methodNames</b>. Note that there
237+
* is no real reason for an ESAPI client to use this, It is intended for
238+
* interal use,
239+
* </p><p>
240+
* The reason this method exists is because certain (other) ESAPI method names
241+
* are considered "unsafe" and therefore should be used with extra caution.
242+
* These "unsafe" methods may include methods that are:
243+
* <ul>
244+
* <li>Deprecated and thus no longer suggested for long term use.</li>
245+
* <li>Methods where the programming contract is not in itself sufficient to ensure safety alone
246+
* and developers are expected to take addional actions on their own to secure their application.</li>
247+
* <li>Methods that are using some unpatched transitive dependency that we haven't firmly
248+
* established grounds for it not being exploitable in the manner that ESAPI uses it.</li>
249+
* <li>Methods whose reference implementations are not scalable to the enterprise level.</li>
250+
* </ul>
251+
* <i>Public</i> methods that are not in that list for the above ESAPI property
252+
* are generally are considered enabled and okay to use unless their Javadoc
253+
* indicates otherwise.
254+
* </p><p>
255+
* Note that this method is intended primarilly for internal ESAPI use and if we were
256+
* using Java Modules (in JDK 9 and later), this method would not be exported.
257+
* </p><p>
258+
* For further details, please see the ESAPI GitHub wiki article,
259+
* <a href="https://github.com/ESAPI/esapi-java-legacy/wiki/Reducing-the-ESAPI-Library's-Attack-Surface">"Reducing the ESAPI Library's Attack Surface"</a>.
260+
* @param fullyQualifiedMethodName A fully qualified ESAPI class name (so, should start
261+
* "org.owasp.esapi.") followed by the method name (but without
262+
* parenthesis or any parameter signature information.
263+
* @return {@code true} if the parameter {@code fullyQualifiedMethodName} is in the comma-separated
264+
* list of values in the ESAPI property <b>ESAPI.dangerouslyAllowUnsafeMethods.methodNames</b>,
265+
* otherwise {@code false} is returned.
266+
*/
267+
public static boolean isMethodExplicityEnabled(String fullyQualifiedMethodName) {
268+
if ( fullyQualifiedMethodName == null || fullyQualifiedMethodName.trim().isEmpty() ) {
269+
throw new IllegalArgumentException("Program error: fullyQualifiedMethodName parameter cannot be null or empty");
270+
}
271+
String desiredMethodName = fullyQualifiedMethodName.trim();
272+
// This regex is too liberal to be anything more than just a trivial
273+
// sanity test to protect against typos.
274+
if ( !desiredMethodName.matches("^org\\.owasp\\.esapi\\.(\\p{Alnum}|\\.)*$") ) {
275+
throw new IllegalArgumentException("Program error: fullyQualifiedMethodName must start with " +
276+
"'org.owasp.esapi.' and be a valid method name.");
277+
}
278+
279+
String enabledMethods = null;
280+
try {
281+
// Need to do this w/in a try/catch because if the property is not
282+
// found, getStringProp will throw a ConfigurationException rather
283+
// than returning a null.
284+
enabledMethods = securityConfiguration().getStringProp("ESAPI.dangerouslyAllowUnsafeMethods.methodNames");
285+
} catch( ConfigurationException cex ) {
286+
return false; // Property not found at all.
287+
}
288+
289+
290+
// Split it up by ',' and then filter it by finding the first on that
291+
// matches the desired method name passed in as the method parameter.
292+
// If no matches, return the empty string.
293+
String result = Arrays.stream( enabledMethods.trim().split(",") )
294+
.filter(methodName -> methodName.trim().equals( desiredMethodName ) )
295+
.findFirst()
296+
.orElse("");
297+
return !result.isEmpty();
298+
}
224299
}

src/main/java/org/owasp/esapi/Encoder.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ public interface Encoder {
472472
* exception ticket to track it.
473473
* </p><p>
474474
* <b>IMPORTANT NOTE:</b> If you really do insist enabling leg cannon mode and use
475-
* this method, then you <i>MUST<i> follow these instructions. Failure to do so will
475+
* this method, then you <i>MUST</i> follow these instructions. Failure to do so will
476476
* result in a {@link org.owasp.esapi.errors.NotConfiguredByDefaultException} being
477477
* thrown when you try to call it. Thus to make it work, you need to add the implementation
478478
* method corresponding to this interace (defined in the property "<b>ESAPI.Encoder</b>"

src/main/java/org/owasp/esapi/errors/NotConfiguredByDefaultException.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,20 +13,22 @@
1313
public class NotConfiguredByDefaultException extends ConfigurationException {
1414

1515
protected static final long serialVersionUID = 1L;
16+
private static final String defaultMsg = "Unknown unsafe ESAPI method invoked without being explicitly allowed. " +
17+
"Check exception stack trace for method name.";
1618

1719
public NotConfiguredByDefaultException(Exception e) {
1820
super(e);
1921
}
2022

2123
public NotConfiguredByDefaultException(String s) {
22-
super(s);
24+
super( (s == null || s.trim().isEmpty()) ? defaultMsg : s);
2325
}
2426

2527
public NotConfiguredByDefaultException(String s, Throwable cause) {
26-
super(s, cause);
28+
super( (s == null || s.trim().isEmpty()) ? defaultMsg : s, cause);
2729
}
2830

2931
public NotConfiguredByDefaultException(Throwable cause) {
30-
super(cause);
32+
super(defaultMsg, cause);
3133
}
3234
}

src/main/java/org/owasp/esapi/reference/DefaultEncoder.java

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@
4545
import org.owasp.esapi.codecs.JSONCodec;
4646
import org.owasp.esapi.errors.EncodingException;
4747
import org.owasp.esapi.errors.IntrusionException;
48+
import org.owasp.esapi.errors.ConfigurationException;
49+
import org.owasp.esapi.errors.NotConfiguredByDefaultException;
50+
51+
import static org.owasp.esapi.PropNames.ACCEPTED_UNSAFE_METHOD_NAMES;
52+
import static org.owasp.esapi.PropNames.ACCEPTED_UNSAFE_METHODS_JUSTIFICATION;
4853

4954

5055
/**
@@ -271,11 +276,75 @@ public String encodeForVBScript(String input) {
271276
return vbScriptCodec.encode(IMMUNE_VBSCRIPT, input);
272277
}
273278

279+
///////////////////////////////////////////////////////////////////////
280+
// TODO - Move this method to some utility class (where?) when we
281+
// are ready to use it on other methods than just encodeForSQL.
282+
//
283+
// At that time, also move the method ESAPI.isMethodExplicityEnabled
284+
// to the same utility class.
285+
/**
286+
* Utility class to throw {@code NotConfiguredByDefaultException} if the
287+
* specified method name is not enabled by default.
288+
*
289+
* @param fullyQualifiedMethodName is the method name that we are checkig if
290+
* enabled in ESAPI.properties.
291+
* @param customAuditMsg is a audit message to log and use in exceptions. If
292+
* this value passed in is {@code null} or the string
293+
* "&lt;default&gt;", then a canned message is used to
294+
* compose the error message.
295+
* @param seeAlso is a string that provides additional reference for context
296+
* such as a CVE ID, GHAS Security Advisory, or ESAPI Security Bulletin.
297+
* @throws NotConfiguredByDefaultException if the specified method name is
298+
* not listed in the property <b>ESAPI.dangerouslyAllowUnsafeMethods.methodNames</b>
299+
* in the <b>ESAPI.properties</b> file.
300+
*/
301+
private void ensureDangerousMethodExplicitlyEnabled(String fullyQualifiedMethodName,
302+
String customAuditMsg,
303+
String seeAlso) {
304+
305+
String auditMsg = null;
306+
if ( customAuditMsg == null || customAuditMsg.equalsIgnoreCase("<default>") ) {
307+
// Special case. Compose an audit message from a canned template.
308+
// TODO: Null / empty check for 'seeAlso'.
309+
auditMsg = "SIEM ALERT: Method '" + fullyQualifiedMethodName + "' has been invoked despite having credible " +
310+
"security concerns; for additional details, see " + seeAlso + ".";
311+
} else {
312+
auditMsg = customAuditMsg; // Use the custom audit message
313+
}
314+
315+
if ( ! ESAPI.isMethodExplicityEnabled( fullyQualifiedMethodName ) ) {
316+
throw new NotConfiguredByDefaultException( "Method not explicitly enabled in property " +
317+
ACCEPTED_UNSAFE_METHOD_NAMES + "; " + auditMsg );
318+
} else {
319+
String justification = null;
320+
try {
321+
// This throws a ConfigurationException (rather than returning null if
322+
// the property name is not found so we need to handle that.
323+
justification = ESAPI.securityConfiguration().getStringProp( ACCEPTED_UNSAFE_METHODS_JUSTIFICATION );
324+
} catch ( ConfigurationException cex ) {
325+
logger.debug( Logger.EVENT_FAILURE, "Property " + ACCEPTED_UNSAFE_METHODS_JUSTIFICATION + " not found.");
326+
justification = "None";
327+
}
328+
329+
if ( justification == null || justification.trim().isEmpty() ) {
330+
justification = "None";
331+
}
332+
logger.warning( Logger.EVENT_FAILURE, auditMsg + " Provided justification: " + justification );
333+
}
334+
return;
335+
}
336+
274337

275338
/**
276339
* {@inheritDoc}
277340
*/
278341
public String encodeForSQL(Codec codec, String input) {
342+
343+
// This will throw if this method is not explicitly enabled in ESAPI.properties.
344+
ensureDangerousMethodExplicitlyEnabled( DefaultEncoder.class.getName() + ".encodeForSQL",
345+
"<default>",
346+
"see CVE-2025-????? and ESAPI Security Bulletin #13 for details" );
347+
279348
if( input == null ) {
280349
return null;
281350
}
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
package org.owasp.esapi;
2+
3+
import org.bouncycastle.crypto.modes.CBCModeCipher;
4+
import org.junit.Assert;
5+
import org.junit.Test;
6+
import org.mockito.Mockito;
7+
import org.owasp.esapi.errors.ConfigurationException;
8+
9+
10+
public class ESAPIVerifyAllowedMethods {
11+
12+
@Test (expected = IllegalArgumentException.class)
13+
public void verifyNulParamThrows() {
14+
ESAPI.isMethodExplicityEnabled(null);
15+
}
16+
17+
@Test (expected = IllegalArgumentException.class)
18+
public void verifyEmptyNoWhitespaceParameterThrows() {
19+
ESAPI.isMethodExplicityEnabled("");
20+
}
21+
22+
@Test (expected = IllegalArgumentException.class)
23+
public void verifyEmptyOnlyWhitespaceParameterThrows() {
24+
ESAPI.isMethodExplicityEnabled(" ");
25+
}
26+
27+
@Test (expected = IllegalArgumentException.class)
28+
public void verifyEmptyOnlyTabWhitespaceParameterThrows() {
29+
ESAPI.isMethodExplicityEnabled("\t");
30+
}
31+
32+
@Test (expected = IllegalArgumentException.class)
33+
public void verifyEmptyOnlyNewlineWhitespaceParameterThrows() {
34+
ESAPI.isMethodExplicityEnabled("\n");
35+
}
36+
37+
38+
39+
@Test (expected = IllegalArgumentException.class)
40+
public void verifyNonEsapiPackageParameterThrows() {
41+
ESAPI.isMethodExplicityEnabled("com.myPackage.myScope.method");
42+
}
43+
@Test
44+
public void verifyUnknownMethodFailsEnableCheck() {
45+
Assert.assertFalse(ESAPI.isMethodExplicityEnabled("org.owasp.esapi.reference.DefaultEncoder.encodeForSQ"));
46+
}
47+
48+
@Test
49+
public void verifyDefinedRestrictionIsCaught() {
50+
Assert.assertTrue(ESAPI.isMethodExplicityEnabled("org.owasp.esapi.reference.DefaultEncoder.encodeForSQL"));
51+
}
52+
53+
@Test
54+
public void testMissingPropertyReturnsFalse() {
55+
try {
56+
SecurityConfiguration mockConfig = Mockito.mock(SecurityConfiguration.class);
57+
Mockito.when(mockConfig.getStringProp("ESAPI.dangerouslyAllowUnsafeMethods.methodNames")).thenThrow(ConfigurationException.class);
58+
ESAPI.override(mockConfig);
59+
60+
Assert.assertFalse(ESAPI.isMethodExplicityEnabled("org.owasp.esapi.thisValueDoesNotMatter"));
61+
Mockito.verify(mockConfig, Mockito.times(1)).getStringProp("ESAPI.dangerouslyAllowUnsafeMethods.methodNames");
62+
} finally {
63+
ESAPI.override(null);
64+
}
65+
66+
}
67+
68+
}

src/test/resources/esapi/ESAPI.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -614,4 +614,4 @@ ESAPI.dangerouslyAllowUnsafeMethods.methodNames=org.owasp.esapi.reference.Defaul
614614
# justification as to why you have enabled these functions. This can be
615615
# anythuing such as a Jira or ServiceNow ticket number, a security exception
616616
# reference, etc. If it is left empty, it will just like "Justification: none".`
617-
ESAPI.enableLegCannonModeAndGetMyAssFired.justification=blah,blah. Please don't fire my @$$. Ticket # 12345
617+
ESAPI.dangerouslyAllowUnsafeMethods.justification=blah,blah. Please don't fire my @$$. Ticket # 12345-not-the-winning-lotto#

0 commit comments

Comments
 (0)