diff --git a/pg/src/main/java/org/bouncycastle/bcpg/ArmoredInputStream.java b/pg/src/main/java/org/bouncycastle/bcpg/ArmoredInputStream.java index 6655b9c7f4..44e7e29622 100644 --- a/pg/src/main/java/org/bouncycastle/bcpg/ArmoredInputStream.java +++ b/pg/src/main/java/org/bouncycastle/bcpg/ArmoredInputStream.java @@ -146,9 +146,11 @@ else if (in3 == '=') boolean restart = false; StringList headerList = Strings.newList(); int lastC = 0; + int lookAhead = -1; boolean isEndOfStream; private boolean validateAllowedHeaders = false; + private boolean csfRejectPrefixedDashes = true; private List allowedHeaders = defaultAllowedHeaders(); /** @@ -199,6 +201,7 @@ private ArmoredInputStream( this.detectMissingChecksum = builder.detectMissingCRC; this.crc = builder.ignoreCRC ? null : new FastCRC24(); this.validateAllowedHeaders = builder.validateAllowedHeaders; + this.csfRejectPrefixedDashes = builder.csfRejectPrefixedDashes; this.allowedHeaders = builder.allowedHeaders; if (hasHeaders) @@ -448,6 +451,12 @@ public int read() if (clearText) { + if (lookAhead != -1) + { + c = lookAhead; + lookAhead = -1; + return c; + } c = in.read(); if (c == '\r' || (c == '\n' && lastC != '\r')) @@ -456,16 +465,26 @@ public int read() } else if (newLineFound && c == '-') { - c = in.read(); - if (c == '-') // a header, not dash escaped + lookAhead = in.read(); + if (lookAhead == '-') // a header, not dash escaped { clearText = false; start = true; restart = true; } - else // a space - must be a dash escape + else if (lookAhead == ' ') // a space - must be a dash escape { + // remove dash escaping c = in.read(); + lookAhead = -1; + } + else + { + // malformed message + if (csfRejectPrefixedDashes) + { + throw new ArmoredInputException("Prefixed dash without trailing space encountered. CSF-signed message malformed."); + } } newLineFound = false; } @@ -683,6 +702,7 @@ public static class Builder private boolean detectMissingCRC = false; private boolean ignoreCRC = false; private boolean validateAllowedHeaders = false; + private boolean csfRejectPrefixedDashes = true; private List allowedHeaders = defaultAllowedHeaders(); private Builder() @@ -709,6 +729,12 @@ public Builder setValidateClearsignedMessageHeaders(boolean validateHeaders) return this; } + public Builder setRejectPrefixedDashesInCSFMessages(boolean rejectDashes) + { + this.csfRejectPrefixedDashes = rejectDashes; + return this; + } + public Builder addAllowedArmorHeader(String header) { allowedHeaders.add(header.trim()); diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/ArmoredInputStreamCSFRejectPrefixedDashTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/ArmoredInputStreamCSFRejectPrefixedDashTest.java new file mode 100644 index 0000000000..ed8bd897b2 --- /dev/null +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/ArmoredInputStreamCSFRejectPrefixedDashTest.java @@ -0,0 +1,75 @@ +package org.bouncycastle.openpgp.test; + +import org.bouncycastle.bcpg.ArmoredInputException; +import org.bouncycastle.bcpg.ArmoredInputStream; +import org.bouncycastle.jce.provider.BouncyCastleProvider; + +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.security.Security; + +import org.bouncycastle.util.test.SimpleTest; +import org.bouncycastle.util.io.Streams; + +public class ArmoredInputStreamCSFRejectPrefixedDashTest + extends SimpleTest { + + public String getName() + { + return "ArmoredInputStreamCSFRejectPrefixedDashTest"; + } + + public static void main(String[] args) + { + Security.addProvider(new BouncyCastleProvider()); + + runTest(new ArmoredInputStreamCSFRejectPrefixedDashTest()); + } + + public void performTest() + throws IOException + { + // signature was created over "payload", but this malformed variant as "-X" prefixed. + String malformed = "-----BEGIN PGP SIGNED MESSAGE-----\n" + + "Hash: SHA512\n" + + "\n" + + "-Xpayload\n" + + "-----BEGIN PGP SIGNATURE-----\n" + + "Version: PGPainless\n" + + "\n" + + "wnUEABYKACcFgmoz+AoJEF0ybVyS+fXHFqEE/9HfXX3exPsb+/QfXTJtXJL59ccA\n" + + "AMmDAP4yxWVmaDycXXgNWuKtyHmWegY+TAQoS2FCrg0KZO/kuQEAnvg8YxQLcL7I\n" + + "WbRs9RZtPLc+jgUKBbz/bode8TkqyQU=\n" + + "=PIAb\n" + + "-----END PGP SIGNATURE-----"; + + ByteArrayInputStream bIn = new ByteArrayInputStream(malformed.getBytes(StandardCharsets.UTF_8)); + ArmoredInputStream aIn = ArmoredInputStream.builder() + .setRejectPrefixedDashesInCSFMessages(true) + .build(bIn); + + try + { + Streams.drain(aIn); + fail("Prefixed dash in CSF message MUST be rejected if configured to do so."); + } + catch (ArmoredInputException e) + { + isEquals("Exception message mismatch", "Prefixed dash without trailing space encountered. CSF-signed message malformed.", e.getMessage()); + } + + bIn = new ByteArrayInputStream(malformed.getBytes(StandardCharsets.UTF_8)); + aIn = ArmoredInputStream.builder() + .setRejectPrefixedDashesInCSFMessages(false) + .build(bIn); + + ByteArrayOutputStream bOut = new ByteArrayOutputStream(); + while (aIn.isClearText()) + { + bOut.write(aIn.read()); + } + isTrue("Malformed payload MUST be returned unaltered", bOut.toString().startsWith("-Xpayload")); + } +} diff --git a/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java b/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java index 3507f450ed..988f86ef34 100644 --- a/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java +++ b/pg/src/test/java/org/bouncycastle/openpgp/test/RegressionTest.java @@ -41,6 +41,7 @@ public class RegressionTest new ArmoredInputStreamTest(), new ArmoredInputStreamBackslashTRVFTest(), new ArmoredInputStreamCRCErrorGetsThrownTest(), + new ArmoredInputStreamCSFRejectPrefixedDashTest(), new ArmoredInputStreamIngoreMissingCRCSum(), new ArmoredOutputStreamTest(), new PGPSessionKeyTest(),