\DEL and C1 controls (U+007F to U+009F) are hex-escaped. Unpaired UTF-16 * surrogates use Unicode escapes; valid surrogate pairs remain unescaped.
@@ -1405,7 +1420,7 @@ public static void forJavaScriptAttribute(Writer out, String input) * *The insertion-boundary requirements and nonempty-fragment HTML * protection of {@link #forJavaScript(String)} apply. This method retains - * the script-letter escapes; its shorter quote escapes do not make it safe + * the terminal script-letter escape; its shorter quote escapes do not make it safe * for HTML event attributes.
* * Example JSP Usage: @@ -1454,8 +1469,8 @@ public static void forJavaScriptBlock(Writer out, String input) * provide surrounding single (') or double (") quotation marks, or backticks * (`) for an ordinary (untagged) template literal. This method * performs the same encode as {@link #forJavaScript(String)} except that - * slash, hyphen, space, {@code !}, {@code <}, {@code >}, both cases of C, - * I, P, R, S, and T, and ampersand are not escaped; double and single + * slash, hyphen, space, {@code !}, {@code <}, {@code >}, terminal C, + * I, P, R, S, and T (either case), and ampersand are not escaped; double and single * quotes use backslash escapes. *DEL and C1 controls (U+007F to U+009F) are hex-escaped. Unpaired UTF-16 * surrogates use Unicode escapes; valid surrogate pairs remain unescaped.
diff --git a/core/src/main/java/org/owasp/encoder/EncodedWriter.java b/core/src/main/java/org/owasp/encoder/EncodedWriter.java index 7661133..978e3ad 100644 --- a/core/src/main/java/org/owasp/encoder/EncodedWriter.java +++ b/core/src/main/java/org/owasp/encoder/EncodedWriter.java @@ -45,7 +45,17 @@ * and close operations synchronize on the wrapped writer. Keep each logical * encoding stream in its own EncodedWriter; synchronization does not prevent * separate callers' content from being interleaved. Close the writer to finish - * pending input; flush alone does not mark the end of input. + * pending input; flush alone does not mark the end of input. In HTML-script + * JavaScript modes, flush safely emits a pending trailing letter from + * {@code script} as a hex escape, or a pending high surrogate as a Unicode escape. + * A later low surrogate is escaped separately, preserving the same JavaScript + * UTF-16 string value. Explicit flushes can therefore add escapes compared with + * encoding the whole value. In these modes, call {@link #flush()} before writing + * directly to the wrapped writer so buffered output and pending input appear + * in order. In other contexts, incomplete sequences can remain pending even + * after flush; do not write a trusted suffix while input remains pending. For a complete + * string followed by trusted output, use the Writer overloads of {@link Encode} + * instead. Closing this writer also closes its wrapped writer. * * @author Jeff Ichnowski */ @@ -234,6 +244,13 @@ private void ensureOpen() throws IOException { public void flush() throws IOException { synchronized (lock) { ensureOpen(); + if (_hasLeftOver && _leftOverBuffer.position() == 1 + && _encoder.canFlushPending(_leftOverBuffer.get(0))) { + // Make a safely representable code unit visible before callers + // write directly to the wrapped writer. Other encoders retain + // their existing incomplete-sequence lookahead behavior. + flushLeftOver(null); + } flushBufferToWriter(); _out.flush(); } diff --git a/core/src/main/java/org/owasp/encoder/Encoder.java b/core/src/main/java/org/owasp/encoder/Encoder.java index 9aaee5c..db8bb1a 100644 --- a/core/src/main/java/org/owasp/encoder/Encoder.java +++ b/core/src/main/java/org/owasp/encoder/Encoder.java @@ -44,8 +44,9 @@ * OWASP Encoder's project is not currently supported. * *All encoder singletons returned by {@link Encoders} are stateless and - * thread-safe. Incomplete sequences remain in the caller's input buffer; - * callers must preserve those characters between calls and must not share + * thread-safe. Characters needing lookahead remain in the caller's input buffer; + * these include incomplete Unicode sequences and trailing HTML-script letters. + * Callers must preserve those characters between calls and must not share * mutable input or output buffers across concurrent calls.
* *To use an Encoder instance directly, repeatedly call {@link @@ -110,7 +111,12 @@ public abstract class Encoder { * true}). This will happen when the encoder needs to see more * input before determining what to do--for example when an encoder sees * a high surrogate at the end of a non-final input buffer and needs the - * next code unit to determine whether it forms a pair.
+ * next code unit to determine whether it forms a pair. HTML-script JavaScript + * encoders also retain a trailing letter from {@code script} until another + * code unit or end-of-input determines whether it needs escaping. After + * {@code UNDERFLOW}, retain unread input and supply more input, or set + * {@code endOfInput} to {@code true} when the logical input is complete; + * retrying the same incomplete input with {@code false} cannot progress. * *The output buffer must have enough remaining space for an entire * encoded sequence. Allow at least 13 characters of output space to support @@ -239,6 +245,18 @@ CoderResult encodeBuffers(CharBuffer input, CharBuffer output, boolean endOfInpu */ abstract int firstEncodedOffset(String input, int off, int len); + /** + * Whether one retained UTF-16 code unit can be represented at an explicit + * writer flush without changing its value when more input follows. Encoders + * that would replace an incomplete sequence must leave it pending instead. + * + * @param ch the sole retained code unit + * @return true if encoding this code unit with endOfInput is safe on flush + */ + boolean canFlushPending(char ch) { + return false; + } + /** * Internal helper method to properly position buffers after encoding up * until an overflow. diff --git a/core/src/main/java/org/owasp/encoder/JavaScriptEncoder.java b/core/src/main/java/org/owasp/encoder/JavaScriptEncoder.java index 0cfb82a..1ef118b 100644 --- a/core/src/main/java/org/owasp/encoder/JavaScriptEncoder.java +++ b/core/src/main/java/org/owasp/encoder/JavaScriptEncoder.java @@ -67,7 +67,7 @@ enum Mode { * prematurely terminating a script block with a closing * "</script" token inside the string. Characters that can contribute * to an HTML script delimiter are escaped so a nonempty encoded fragment - * cannot complete a raw token supplied partly by trusted literal text. + * cannot complete a token supplied partly by trusted literal text. * The surrounding template must satisfy the insertion-boundary * requirements documented by Encode.forJavaScript. */ @@ -101,6 +101,11 @@ enum Mode { * characters that would normally not be encoded, will be encoded. */ private final boolean _asciiOnly; + /** + * True if HTML script tokens split across insertion boundaries must be + * prevented in addition to JavaScript string syntax. + */ + private final boolean _htmlScript; /** * Constructs a new JavaScriptEncoder for the specified contextual mode. @@ -116,6 +121,7 @@ enum Mode { // (to be clear, it's much faster on 32-bit VMS, and just slightly // faster on 64-bit VMS) _mode = mode; + _htmlScript = mode == Mode.BLOCK || mode == Mode.HTML; // Note: this probably needs to be repeated everywhere this trick is // used, but here seems like as good a place as any. According to @@ -133,23 +139,22 @@ enum Mode { -1 & ~((1 << '\\')), ~((1 << Unicode.DEL) | (1 << '`') | (1 << '{')),}; - if (mode == Mode.BLOCK || mode == Mode.HTML) { + if (_htmlScript) { // In ", "\\x3c\\/script\\x3e"}, + {"", "\\x3c\\x21\\x2d\\x2d\\x20\\x2d\\x2d\\x3e"}, + {"", ""}, + {null, "null"} + }; + for (int mode = 0; mode < ENCODERS.length; mode++) { + for (String[] sample : samples) { + assertEquals(CONTEXTS[mode], sample[1], facade(mode, sample[0])); + StringWriter output = new StringWriter(); + if (mode == 0) { + Encode.forJavaScript(output, sample[0]); + } else { + Encode.forJavaScriptBlock(output, sample[0]); + } + assertEquals(CONTEXTS[mode], sample[1], output.toString()); + assertEquals(CONTEXTS[mode], sample[1], + Encode.encode(Encoders.forName(CONTEXTS[mode]), sample[0])); + } + } + assertEquals("http://google.com", Encode.forJavaScriptSource("http://google.com")); + assertEquals("http://google.com", Encode.forJavaScriptAttribute("http://google.com")); + } + + public void testEveryTerminalScriptLetterIsEscapedOnlyInScriptModes() { + for (char ch : "CIPRSTciprst".toCharArray()) { + String input = "a" + ch; + String expected = "a\\x" + Integer.toHexString(ch); + assertEquals(expected, Encode.forJavaScript(input)); + assertEquals(expected, Encode.forJavaScriptBlock(input)); + assertEquals(input, Encode.forJavaScriptSource(input)); + assertEquals(input, Encode.forJavaScriptAttribute(input)); + } + } + + public void testFastPathRespectsSubstringEndAndOffsets() { + for (Encoder encoder : ENCODERS) { + assertEquals(9, encoder.firstEncodedOffset("scripture", 0, 9)); + assertEquals(5, encoder.firstEncodedOffset("script", 0, 6)); + assertEquals(6, encoder.firstEncodedOffset("xScriptz", 1, 6)); + assertEquals(10, encoder.firstEncodedOffset("xscriptureY", 1, 9)); + assertEquals(1, encoder.firstEncodedOffset("xt!x", 1, 1)); + assertEquals(2, encoder.firstEncodedOffset("xs!x", 1, 2)); + assertEquals(2, encoder.firstEncodedOffset("xx", 2, 0)); + } + String unchanged = new String("scripture"); + assertSame(unchanged, Encode.forJavaScript(unchanged)); + assertSame(unchanged, Encode.forJavaScriptBlock(unchanged)); + } + + public void testFacadeBuffersDoNotCreateAdditionalTerminalEscapes() throws Exception { + for (int mode = 0; mode < ENCODERS.length; mode++) { + for (int size : new int[]{1, 1023, 1024, 1025, 2047, 2048, 2049, 8193}) { + String prefix = repeat('a', size - 1); + assertEquals(prefix + "\\x74", facade(mode, prefix + "t")); + String input = "!" + prefix + "ta"; + String expected = "\\x21" + prefix + "ta"; + assertEquals(expected, facade(mode, input)); + StringWriter output = new StringWriter(); + if (mode == 0) { + Encode.forJavaScript(output, input); + } else { + Encode.forJavaScriptBlock(output, input); + } + assertEquals(expected, output.toString()); + } + String outputOverflow = repeat('!', 2048) + "scripture"; + assertEquals(repeatEncoded("\\x21", 2048) + "scripture", + facade(mode, outputOverflow)); + } + } + + public void testLowLevelRetainsOneLetterUntilItKnowsTheInputEnds() { + for (Encoder encoder : ENCODERS) { + for (char ch : "CIPRSTciprst".toCharArray()) { + CharBuffer input = CharBuffer.allocate(2); + input.put(ch).flip(); + CharBuffer output = CharBuffer.allocate(16); + assertTrue(encoder.encode(input, output, false).isUnderflow()); + assertEquals(0, input.position()); + assertEquals(0, output.position()); + input.compact(); + input.put('a').flip(); + assertTrue(encoder.encode(input, output, false).isUnderflow()); + assertFalse(input.hasRemaining()); + output.flip(); + assertEquals(ch + "a", output.toString()); + + input.clear(); + input.put(ch).flip(); + output.clear(); + assertTrue(encoder.encode(input, output, false).isUnderflow()); + assertTrue(encoder.encode(input, output, true).isUnderflow()); + assertFalse(input.hasRemaining()); + output.flip(); + assertEquals("\\x" + Integer.toHexString(ch), output.toString()); + } + } + } + + public void testTerminalEscapeOverflowIsAtomic() { + for (Encoder encoder : ENCODERS) { + for (int capacity = 0; capacity < 4; capacity++) { + CharBuffer input = CharBuffer.wrap(new char[]{'^', '^', 's', '^'}); + input.position(2).limit(3); + char[] guarded = new char[capacity + 4]; + Arrays.fill(guarded, '^'); + CharBuffer output = CharBuffer.wrap(guarded); + output.position(2).limit(capacity + 2); + assertTrue(encoder.encode(input, output, true).isOverflow()); + assertEquals(2, input.position()); + assertEquals(2, output.position()); + for (char ch : guarded) { + assertEquals('^', ch); + } + output = CharBuffer.allocate(4); + assertTrue(encoder.encode(input, output, true).isUnderflow()); + assertFalse(input.hasRemaining()); + output.flip(); + assertEquals("\\x73", output.toString()); + } + } + } + + public void testChunkedSlicesAndOutputOverflowsMatchFacade() { + for (int mode = 0; mode < ENCODERS.length; mode++) { + String input = "scripture script>/cCatP"; + for (int chunk = 1; chunk <= input.length(); chunk++) { + for (int capacity = 4; capacity <= 7; capacity++) { + assertEquals("chunk=" + chunk + ", capacity=" + capacity, + facade(mode, input), lowLevel(ENCODERS[mode], input, chunk, capacity)); + } + } + input = "s\u2028c\ud83d\ude00i"; + for (int chunk = 1; chunk <= input.length(); chunk++) { + assertEquals(facade(mode, input), lowLevel(ENCODERS[mode], input, chunk, 6)); + } + } + } + + public void testEncodedWriterEverySplitMatchesFacade() throws Exception { + for (int mode = 0; mode < ENCODERS.length; mode++) { + for (String input : new String[]{"", "s", "script", "scripture", + "http://google.com", "", "s\u2028c\ud83d\ude00i"}) { + for (int split = 0; split <= input.length(); split++) { + assertWriterSplit(mode, input, split); + } + } + String large = "!" + repeat('a', 1022) + "scripture" + repeat('!', 1024) + "t"; + for (int split : new int[]{0, 1, 1023, 1024, 1025, 1032, + large.length() - 1, large.length()}) { + assertWriterSplit(mode, large, split); + } + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, CONTEXTS[mode]); + for (char ch : large.toCharArray()) { + writer.write(ch); + } + writer.close(); + assertEquals(facade(mode, large), output.toString()); + } + } + + public void testFlushMakesTerminalLetterVisibleWithoutEndingTheStream() throws Exception { + for (String context : CONTEXTS) { + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write("cat"); + writer.flush(); + writer.flush(); + writer.write(""); + writer.flush(); + assertEquals("ca\\x74", output.toString()); + writer.write("a"); + writer.flush(); + assertEquals("ca\\x74a", output.toString()); + writer.close(); + assertEquals("ca\\x74a", output.toString()); + + output = new StringWriter(); + writer = new EncodedWriter(output, context); + writer.write("cat"); + writer.flush(); + assertEquals("ca\\x74", output.toString()); + writer.close(); + assertEquals("ca\\x74", output.toString()); + } + } + + public void testFlushEscapesAllScriptLettersAndLeavesOtherModesUnchanged() throws Exception { + for (String context : CONTEXTS) { + for (char ch : "CIPRSTciprst".toCharArray()) { + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write(ch); + writer.flush(); + assertEquals("\\x" + Integer.toHexString(ch), output.toString()); + writer.write('a'); + writer.close(); + assertEquals("\\x" + Integer.toHexString(ch) + "a", output.toString()); + } + } + for (String context : new String[]{Encoders.JAVASCRIPT_ATTRIBUTE, Encoders.JAVASCRIPT_SOURCE}) { + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write("script"); + writer.flush(); + assertEquals("script", output.toString()); + writer.close(); + assertEquals("script", output.toString()); + } + } + + public void testHtmlScriptFlushRepresentsSurrogatesWithoutChangingTheirValue() throws Exception { + for (String context : CONTEXTS) { + for (String pair : new String[]{"\ud800\udc00", "\ud83d\ude00", "\udbff\udfff"}) { + String highEscape = "\\u" + Integer.toHexString(pair.charAt(0)); + String pairEscape = highEscape + "\\u" + Integer.toHexString(pair.charAt(1)); + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write(pair.charAt(0)); + writer.flush(); + writer.flush(); + assertEquals(context, highEscape, output.toString()); + writer.write(pair.charAt(1)); + writer.close(); + assertEquals(context, pairEscape, output.toString()); + + output = new StringWriter(); + writer = new EncodedWriter(output, context); + writer.write(pair.charAt(0)); + writer.write(pair.charAt(1)); + writer.close(); + assertEquals("no explicit flush keeps raw pair: " + context, pair, output.toString()); + } + } + } + + public void testFlushRetainsSurrogateLookaheadInOtherContexts() throws Exception { + for (String context : new String[]{Encoders.JAVASCRIPT_ATTRIBUTE, Encoders.JAVASCRIPT_SOURCE, + Encoders.CDATA, Encoders.XML_COMMENT}) { + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write('\ud83d'); + writer.flush(); + writer.flush(); + assertEquals(context, "", output.toString()); + writer.write('\ude00'); + writer.close(); + assertEquals(context, "\ud83d\ude00", output.toString()); + } + } + + public void testFlushHandlesInsufficientSpaceForTheCompletePendingEscape() throws Exception { + for (String context : CONTEXTS) { + for (String[] unit : new String[][]{{"s", "\\x73"}, {"\ud83d", "\\ud83d"}}) { + for (int available = 0; available < unit[1].length(); available++) { + String prefix = repeat('a', EncodedWriter.BUFFER_SIZE - available); + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write(prefix + unit[0]); + writer.flush(); + assertEquals(prefix + unit[1], output.toString()); + writer.write('a'); + writer.close(); + assertEquals(prefix + unit[1] + "a", output.toString()); + } + } + } + } + + public void testFlushBeforeDirectTrustedSuffixCannotReorderALetterIntoAnEndTag() throws Exception { + String prefix = "
after
"; + Document intended = Jsoup.parse(prefix + "i" + suffix); + assertNull("unencoded intended value has a different tag name", intended.getElementById("pwn")); + Document reordered = Jsoup.parse(prefix + suffix); + assertNotNull("negative control: omitting the pending letter closes script", + reordered.getElementById("pwn")); + for (String context : CONTEXTS) { + StringWriter output = new StringWriter(); + output.write(prefix); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write('i'); + writer.flush(); + output.write(suffix); + writer.close(); + Document parsed = Jsoup.parse(output.toString()); + assertNull(context, parsed.getElementById("pwn")); + assertEquals(context, 1, parsed.select("script").size()); + assertEquals(context, "var value=\"after
"; + Document intended = Jsoup.parse(prefix + "i\ud800" + suffix); + assertNull("surrogate prevents the intended value from matching script", + intended.getElementById("pwn")); + Document reordered = Jsoup.parse(prefix + "i" + suffix); + assertNotNull("negative control: withholding the surrogate closes script", + reordered.getElementById("pwn")); + for (String context : CONTEXTS) { + StringWriter output = new StringWriter(); + output.write(prefix); + EncodedWriter writer = new EncodedWriter(output, context); + writer.write("i\ud800"); + writer.flush(); + output.write(suffix); + writer.close(); + Document parsed = Jsoup.parse(output.toString()); + assertNull(context, parsed.getElementById("pwn")); + assertEquals(context, 1, parsed.select("script").size()); + assertEquals(context, "var value=\"after
"); + assertEquals(label, 1, parsed.select("script").size()); + assertEquals(label, source, parsed.selectFirst("script").data()); + assertNotNull(label, parsed.getElementById("after")); + assertEquals(label, "after", parsed.getElementById("after").text()); + } + + private static String repeat(char ch, int count) { + char[] value = new char[count]; + Arrays.fill(value, ch); + return new String(value); + } + + private static String repeatEncoded(String value, int count) { + StringBuilder result = new StringBuilder(value.length() * count); + for (int i = 0; i < count; i++) { + result.append(value); + } + return result.toString(); + } + + private static void assertWriterSplit(int mode, String input, int split) throws Exception { + StringWriter output = new StringWriter(); + EncodedWriter writer = new EncodedWriter(output, CONTEXTS[mode]); + writer.write(input.substring(0, split)); + writer.write(input.substring(split)); + writer.close(); + assertEquals("split=" + split, facade(mode, input), output.toString()); + } + + private static String lowLevel(Encoder encoder, String value, int chunk, int capacity) { + char[] guardedInput = new char[value.length() + 4]; + Arrays.fill(guardedInput, '^'); + value.getChars(0, value.length(), guardedInput, 2); + CharBuffer parent = CharBuffer.wrap(guardedInput); + parent.position(1).limit(value.length() + 3); + CharBuffer input = parent.slice(); + input.position(1).limit(1); + StringBuilder encoded = new StringBuilder(); + int end = 1; + while (true) { + end = Math.min(end + chunk, value.length() + 1); + input.limit(end); + boolean last = end == value.length() + 1; + while (true) { + char[] guardedOutput = new char[capacity + 4]; + Arrays.fill(guardedOutput, '^'); + parent = CharBuffer.wrap(guardedOutput); + parent.position(1).limit(capacity + 3); + CharBuffer output = parent.slice(); + output.position(1).limit(capacity + 1); + CoderResult result = encoder.encode(input, output, last); + encoded.append(guardedOutput, 2, output.position() - 1); + assertEquals('^', guardedOutput[0]); + assertEquals('^', guardedOutput[1]); + for (int i = output.position() + 1; i < guardedOutput.length; i++) { + assertEquals('^', guardedOutput[i]); + } + if (result.isUnderflow()) { + break; + } + assertTrue(result.isOverflow()); + assertTrue(output.position() > 1); + } + if (last) { + assertFalse(input.hasRemaining()); + assertEquals('^', guardedInput[0]); + assertEquals('^', guardedInput[1]); + assertEquals('^', guardedInput[value.length() + 2]); + assertEquals('^', guardedInput[value.length() + 3]); + return encoded.toString(); + } + } + } +} diff --git a/core/src/test/java/org/owasp/encoder/JavaScriptEncoderTest.java b/core/src/test/java/org/owasp/encoder/JavaScriptEncoderTest.java index 00694f9..6321a4b 100644 --- a/core/src/test/java/org/owasp/encoder/JavaScriptEncoderTest.java +++ b/core/src/test/java/org/owasp/encoder/JavaScriptEncoderTest.java @@ -53,15 +53,9 @@ public static Test suite() { for (JavaScriptEncoder.Mode mode : JavaScriptEncoder.Mode.values()) { // if (!(mode == JavaScriptEncoder.Mode.HTML_CONTENT && asciiOnly == 0)) continue; boolean htmlBlock = mode == Mode.BLOCK || mode == Mode.HTML; - String trustedDollarExpected = htmlBlock - ? "\\x7bexe\\x63u\\x74ed=\\x74\\x72ue}" - : "\\x7bexecuted=true}"; - String templateExpressionExpected = htmlBlock - ? "\\x24\\x7bale\\x72\\x74(1)}" - : "\\x24\\x7balert(1)}"; - String templateBreakoutExpected = htmlBlock - ? "hell\\x60;ale\\x72\\x74(1);\\x60o" - : "hell\\x60;alert(1);\\x60o"; + String trustedDollarExpected = "\\x7bexecuted=true}"; + String templateExpressionExpected = "\\x24\\x7balert(1)}"; + String templateBreakoutExpected = "hell\\x60;alert(1);\\x60o"; EncoderTestSuiteBuilder builder = new EncoderTestSuiteBuilder( new JavaScriptEncoder(mode, asciiOnly==1), "(xyz)", "(\\)") .encoded(0, 0x1f) @@ -96,8 +90,7 @@ public static Test suite() { .encode("\\x3c", "<") .encode("\\x3e", ">") .encode("script delimiter alphabet", - "\\x43\\x49\\x50\\x52\\x53\\x54" - + "\\x63\\x69\\x70\\x72\\x73\\x74", + "CIPRSTciprs\\x74", "CIPRSTciprst", "CIPRSTciprst") .encoded(" !-/<>CIPRSTciprst"); break; @@ -136,8 +129,8 @@ public static Test suite() { "${alert(1)}") .encode("template breakout", templateBreakoutExpected, "hell`;alert(1);`o") - .encode("abc", htmlBlock ? "ab\\x63" : "abc", "abc") - .encode("ABC", htmlBlock ? "AB\\x43" : "ABC", "ABC") + .encode("abc", htmlBlock ? "ab\\x63" : "abc", "abc", "abc") + .encode("ABC", htmlBlock ? "AB\\x43" : "ABC", "ABC", "ABC") // DEL and the C1 controls are hex encoded in every mode .encode("DEL", "\\x7f", "\u007f") .encode("U+0080", "\\x80", "\u0080") @@ -177,15 +170,13 @@ public static Test suite() { public void testTemplateCharactersThroughPublicFacadesAndRegistry() throws Exception { String input = "price=$5;`${value}`;\\${escaped}"; String expected = "price=\\x245;\\x60\\x24\\x7bvalue}\\x60;\\\\\\x24\\x7bescaped}"; - String htmlBlockExpected = "\\x70\\x72\\x69\\x63e=\\x245;" - + "\\x60\\x24\\x7bvalue}\\x60;\\\\\\x24\\x7be\\x73\\x63a\\x70ed}"; - String trustedDollarExpected = "\\x7bexe\\x63u\\x74ed=\\x74\\x72ue}"; + String trustedDollarExpected = "\\x7bexecuted=true}"; String[] methods = {"forJavaScript", "forJavaScriptAttribute", "forJavaScriptBlock", "forJavaScriptSource"}; String[] contexts = {Encoders.JAVASCRIPT, Encoders.JAVASCRIPT_ATTRIBUTE, Encoders.JAVASCRIPT_BLOCK, Encoders.JAVASCRIPT_SOURCE}; for (int i = 0; i < methods.length; i++) { - String methodExpected = (i == 0 || i == 2) ? htmlBlockExpected : expected; + String methodExpected = expected; assertEquals(methods[i], methodExpected, Encode.class.getMethod(methods[i], String.class).invoke(null, input)); StringWriter out = new StringWriter(); @@ -193,8 +184,7 @@ public void testTemplateCharactersThroughPublicFacadesAndRegistry() throws Excep assertEquals(methods[i], methodExpected, out.toString()); assertEquals(contexts[i], methodExpected, Encode.encode(Encoders.forName(contexts[i]), input)); - assertEquals(methods[i], (i == 0 || i == 2) - ? trustedDollarExpected : "\\x7bexecuted=true}", + assertEquals(methods[i], trustedDollarExpected, Encode.class.getMethod(methods[i], String.class).invoke(null, "{executed=true}")); } } diff --git a/core/src/test/java/org/owasp/encoder/JavaScriptUnicodeTest.java b/core/src/test/java/org/owasp/encoder/JavaScriptUnicodeTest.java index b47c27d..5ad363a 100644 --- a/core/src/test/java/org/owasp/encoder/JavaScriptUnicodeTest.java +++ b/core/src/test/java/org/owasp/encoder/JavaScriptUnicodeTest.java @@ -141,16 +141,17 @@ public void testEncodedWriterLookaheadAcrossFullOutputBuffer() throws Exception } } - public void testFlushDoesNotFinalizePendingPair() throws Exception { + public void testFlushUsesValuePreservingSurrogateEscapesOnlyInScriptModes() throws Exception { for (JavaScriptEncoder.Mode mode : JavaScriptEncoder.Mode.values()) { StringWriter out = new StringWriter(); EncodedWriter writer = new EncodedWriter(out, new JavaScriptEncoder(mode, false)); writer.write("a\ud83d"); writer.flush(); - assertEquals("a", out.toString()); + boolean script = mode == JavaScriptEncoder.Mode.HTML || mode == JavaScriptEncoder.Mode.BLOCK; + assertEquals(script ? "a\\ud83d" : "a", out.toString()); writer.write("\ude00"); writer.close(); - assertEquals("a\ud83d\ude00", out.toString()); + assertEquals(script ? "a\\ud83d\\ude00" : "a\ud83d\ude00", out.toString()); } } diff --git a/docs/compatibility-decisions.md b/docs/compatibility-decisions.md index 5a0561a..7a180f1 100644 --- a/docs/compatibility-decisions.md +++ b/docs/compatibility-decisions.md @@ -43,11 +43,11 @@ remain outside those contracts. ## 1.5 parser-boundary output migration -The 1.5 security correction keeps the existing trusted-prefix/encoded-value/ -trusted-suffix composition contract. Applications do not need a new stateful +The 1.5 security correction makes the trusted-prefix/encoded-value/ +trusted-suffix protection explicit. Applications do not need a new stateful API, but three outputs change compared with 1.4.1: -| Context | 1.5 behavior | Compatibility consequence | +| Context | 1.5.0–1.5.1 behavior | Compatibility consequence | | --- | --- | --- | | HTML script JavaScript (`forJavaScript`, `forJavaScriptBlock`) | Escapes every character that can contribute to case-insensitive ``; hyphen uses `\x2d` | JavaScript string values are preserved, but common letters and spaces can change encoded bytes. Attribute-only and standalone-source modes retain their narrower rules. | | XML CDATA | Represents every `]` and `>` with a close/character-data/reopen spelling | Parsed XML text is preserved, apart from documented XML normalization/replacement. Output can expand 13× and parser event boundaries can change. | @@ -66,6 +66,41 @@ the CDATA maximum is 13×. Large CDATA values should use a Writer facade or `EncodedWriter`; String facades retain the complete result but grow according to actual output rather than reserving the maximum bound. +## Unreleased JavaScript output refinement (#243) + +Release recommendation: use a minor release (1.6.0), rather than assuming this +is patch-compatible. The evaluated values and supported HTML tokenizer protection +remain intact, but encoded bytes, explicit-flush spelling, and the previous literal +substring prohibition change. This proposal records the migration and parser +validation requirements; the current development snapshot version is not release +authorization. Existing published artifacts remain unchanged. + +The script-fragment protection remains, with less escaping of ordinary letters. +HTML punctuation and whitespace still use the 1.5 escapes. Of the letters in +`script` (either case), only the final character of an encoded input is escaped. +For example, `i` still becomes `\x69`, `script` becomes `scrip\x74`, and +`http://google.com` becomes `http:\/\/google.com`. Attribute-only and standalone +source modes are unchanged. This is conservative rather than minimal encoding: +`cat` still becomes `ca\x74`. The terminal rule keeps the streaming implementation +stateless and its safety argument small. + +The guarantee concerns recognized HTML script tokens, not every raw substring +spelling `` cannot close a script. +The trusted-boundary requirements above still apply, including the empty-input +and already-started-end-tag exclusions. This changes encoded bytes, not the +JavaScript string value or supported embedding contexts. + +The encoder remains stateless. A streaming call retains a trailing script letter +until more input or end-of-input determines whether it needs escaping. +`EncodedWriter.flush()` safely emits a pending letter as a hex escape or a pending +high surrogate as a Unicode escape before flushing the wrapped writer. A later +low surrogate is escaped separately, preserving the same JavaScript UTF-16 value. This avoids withholding ordinary letters when callers +flush before subsequent output. Explicit flushes can add safe escapes compared with +the whole-value facade; ordinary write chunking without flushes produces identical +bytes. In other encoding contexts, incomplete sequences can remain pending until +more input or `close()`, which also closes the wrapped writer. Prefer the complete-value Writer facade when +trusted output follows. + ## Base64url disposition (#149) **Reject adding a Base64Url class, `Encode.forBase64Url`, or tag/EL bindings to this diff --git a/docs/contexts.md b/docs/contexts.md index cfc4e9e..788d03d 100644 --- a/docs/contexts.md +++ b/docs/contexts.md @@ -1,8 +1,9 @@ # Output contexts and boundaries -This guide describes **1.5.1**, which clarifies the unchanged 1.5.0 encoding -behavior; feature introductions are marked below. See the -[release record](../releases/1.5.1.md) for publication status. +This guide describes the current development version, including the unreleased +JavaScript refinement for #243. Published 1.5.0 and 1.5.1 retain their broader +letter escapes; 1.5.1 clarified the 1.5.0 behavior without changing it. See the +[1.5.1 release record](../releases/1.5.1.md) for publication status. The [Encode Javadoc source](../core/src/main/java/org/owasp/encoder/Encode.java) is the detailed per-method contract; each method has a String-returning and a `(Writer out, String input)` overload. `Encoders` exposes shared stateless encoders; @@ -78,15 +79,37 @@ escapes, preserving JavaScript string values through UTF-8 output. Valid surroga pairs remain intact. This does not promise that every downstream system accepts unpaired surrogates. -The general and block encoders also escape every ASCII character that can form -part of case-insensitive `` -script tokens. This keeps the encoding of any nonempty value from completing a delimiter -supplied partly by adjacent trusted literal string text. The additional output -changes include space, `!`, hyphen (`\x2d`), `<`, `>`, slash, and both cases of -the letters in `script`. JavaScript interprets these spellings as the original -string value, but byte-for-byte output, snapshots, signatures and cache keys can -change. `forJavaScriptAttribute` and `forJavaScriptSource` do not apply the HTML -raw-text rule because their output is not for an HTML script element. +The general and block encoders prevent a nonempty encoded value from completing +recognized `` with adjacent trusted literal text. They escape +space, `!`, hyphen (`\x2d`), `<`, `>`, slash, and relevant controls throughout +the input. In the unreleased refinement for #243, letters from `script` (either case) are escaped +only when they are the final input character. Interior letters remain readable; +a longer name such as `` is not a script end tag. This preserves +HTML tokenizer containment while allowing harmless raw `` still needs the letter escape. This is unrelated to whitespace inside a +tag name. The terminal-letter rule is conservative: both `script` and `csript` +end with `t`, so both receive a final `\x74` escape even though `csript` is not +a script tag name. + +JavaScript interprets the escapes as the original string value, but encoded bytes, +snapshots, signatures and cache keys change. `forJavaScriptAttribute` and +`forJavaScriptSource` do not apply the HTML raw-text rule because their output is +not for an HTML script element. A streaming encoder may retain a final script +letter until more input or end-of-input. `EncodedWriter.flush()` safely emits a +pending script letter as a hex escape or a pending high surrogate as a Unicode +escape. A later low surrogate is escaped separately. Explicit flushes can add +escapes without changing the JavaScript UTF-16 value. Other encoding contexts +retain their existing incomplete-input handling. +In general/block JavaScript modes, flush the `EncodedWriter` before writing +directly to its wrapped writer. +Use the complete-value Writer facade when trusted output must follow. The opening `after
'+ + '','text/html'); + const subject=doc.getElementById('subject'); + const after=doc.getElementById('after'); + const tail=doc.getElementById('tail'); + // HTML normalizes raw CR/CRLF before tokenization. Encoded CR stays escaped. + const exact=subject && subject.textContent===source.replace(/\\r\\n?/g,'\\n'); + const contained=!!(exact && doc.scripts.length===2 && + subject.parentElement===doc.body && doc.body.childElementCount===3 && + after && after.textContent==='after' && after.previousElementSibling===subject && + tail && after.nextElementSibling===tail && tail.textContent==='/* tail */'); + if (contained!==expected && failures.length<8) { + failures.push([label,expected,contained,doc.scripts.length, + subject ? subject.textContent.slice(0,250) : null]); + } + } + return failures; + """; + private static final String VERIFY_VALUE_BATCH = """ + const failures=[]; + for (const [label,source,metadata] of arguments[0]) { + const [input,mode,quote]=metadata; + try { + const sources=[source]; + if (mode===0 || mode===2) { + const doc=new DOMParser().parseFromString( + 'after
','text/html'); + if (doc.scripts.length!==1 || !doc.getElementById('after') || + doc.scripts[0].textContent!==source) throw Error('script containment'); + sources.push(doc.scripts[0].textContent); + } + if ((mode===0 || mode===1) && quote==='`') { + for (const outer of ["'",'"']) { + const doc=new DOMParser().parseFromString( + '','text/html'); + sources.push(doc.querySelector('button').getAttribute('onclick')); + } + } + for (const candidate of sources) { + const result=new Function(candidate)(); + if (result[0]!==input || result[1]!==false) throw Error('value/execution changed'); + } + } catch (error) { + if (failures.length<8) failures.push([label,String(error)]); + } + } + return failures; + """; + private static final String VERIFY_LIVE_FRAMES = """ + const fixtures=arguments[0],done=arguments[arguments.length-1],failures=[]; + let next=0; + async function worker() { + while (nextafter
'+ + ''; + document.body.append(frame); + }); + } + } + Promise.all(Array.from({length:8},worker)).then(()=>done([fixtures.length,failures])) + .catch(error=>done([-1,String(error)])); + """; private static final String[] INPUTS = { "", "plain", "$", "`", "{executed=true}", "${", "${executed=true}", "hell`;executed=true;value=`o", "\\${executed=true}", "\\`", "end$", @@ -177,6 +290,393 @@ void preservesEveryLoneSurrogateAndControlThroughUtf8AndJavaScriptParsing() thro } } + @Test + void containsEveryNonemptyPartitionOfEveryCaseAndDelimiterOfScriptNames() { + BrowserBatch batch = new BrowserBatch(VERIFY_HTML_BATCH); + for (int mode = 0; mode < SCRIPT_ENCODERS.size(); mode++) { + UnaryOperator