Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CVE-2021-28170 Fix expression delimiter escaping #160

Merged
merged 1 commit into from
Aug 16, 2021

Conversation

TomasHofman
Copy link
Contributor

Co-authored-by: rmartinc [email protected]

Fixes #155

Copy link

@pdudits pdudits left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

@AppSecConcierge
Copy link

Any ETA on when this fix will be released into a new version?

@eskimopip
Copy link

+1 for a merge and new version

@dantran
Copy link

dantran commented Aug 14, 2021

Can we have this fix into 3.x? it is not possible for us to upgrade 4.x due to packaging changes

@dantran
Copy link

dantran commented Aug 14, 2021

Copy link

@m0mus m0mus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@jbescos jbescos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@m0mus
Copy link

m0mus commented Aug 16, 2021

There are missing copyright headers. I'll merge it and we'll add it later respecting the authorship.

@m0mus m0mus merged commit fe0c60b into jakartaee:master Aug 16, 2021
@jbescos jbescos mentioned this pull request Aug 16, 2021
@mmkamburova
Copy link

We need this fix as well in v3.x as it'll be impossible at the moment to upgrade to v4.x. Could you please let us know whether you're planning to port it in v3.x and provide an ETA if so?

Thanks,
Maria

el-bot pushed a commit that referenced this pull request Aug 17, 2021
senivam pushed a commit to senivam/el-ri that referenced this pull request Aug 17, 2021
m0mus pushed a commit that referenced this pull request Aug 17, 2021
* CVE-2021-28170 Fix expression delimiter escaping (#160)

Co-authored-by: rmartinc [email protected]

* Fix copyrights (#164)

Signed-off-by: Jorge Bescos Gascon <[email protected]>

* javax adaptation

Signed-off-by: Maxim Nesen <[email protected]>

Co-authored-by: TomasHofman <[email protected]>
Co-authored-by: jbescos <[email protected]>
@deepmanit
Copy link

is it merged in version 3.0.3 or we need to upgrade for 4.x. to fix the issue

@joschi
Copy link

joschi commented Aug 18, 2021

@deepmanit The changes are in https://github.com/eclipse-ee4j/el-ri/releases/tag/3.0.4-impl but it hasn't been published to Maven Central yet.

@Sachpat
Copy link

Sachpat commented Sep 21, 2021

@deepmanit The changes are in https://github.com/eclipse-ee4j/el-ri/releases/tag/3.0.4-impl but it hasn't been published to Maven Central yet.

@joschi any idea when will the 3.x be pushed to Maven central? It would be really helpful if we get it ASAP in Maven Central :)

@fenneclabs
Copy link

@deepmanit The changes are in https://github.com/eclipse-ee4j/el-ri/releases/tag/3.0.4-impl but it hasn't been published to Maven Central yet.

@joschi any idea when will the 3.x be pushed to Maven central? It would be really helpful if we get it ASAP in Maven Central :)

It's been there for a month now: https://search.maven.org/artifact/com.sun.el/el-ri/3.0.4/jar

@Sachpat
Copy link

Sachpat commented Sep 21, 2021

@deepmanit The changes are in https://github.com/eclipse-ee4j/el-ri/releases/tag/3.0.4-impl but it hasn't been published to Maven Central yet.

@joschi any idea when will the 3.x be pushed to Maven central? It would be really helpful if we get it ASAP in Maven Central :)

It's been there for a month now: https://search.maven.org/artifact/com.sun.el/el-ri/3.0.4/jar

@fenneclabs but I don't see jakarta.el:jakarta.el-api:3.0.4 in maven central. Am I missing something here?

@markt-asf
Copy link
Contributor

@fenneclabs but I don't see jakarta.el:jakarta.el-api:3.0.4 in maven central. Am I missing something here?

Yes. The API and the implementation are separate and are provided in separate JARs. This is a vulnerability in the Glassfish implementation of the EL API so the Glassfish implementation was updated. The EL API is unaffected.

hankem added a commit to TNG/config-builder that referenced this pull request Sep 10, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 10, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 11, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 12, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 15, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 18, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
hankem added a commit to TNG/config-builder that referenced this pull request Sep 23, 2022
After `javax.el` v. 3.0.1-b*, the artifactId was changed to `jakarta.el`,
cf. https://github.com/javaee/uel-ri:
> This project is now part of the EE4J initiative.
> This repository has been archived
> as all activities are now happening in the corresponding Eclipse repository.

`jakarta.el` v. <= 3.0.3 (and likewise previous versions of `javax.el`)
suffer from https://nvd.nist.gov/vuln/detail/CVE-2021-28170.
Its effect can be seen in the following unit tests
(after jakartaee/expression-language#160):

```
    @test
    public void testEscape01() {
        testEvaluation("$${1+1}", "$2");
        testEvaluation("$\\${1+1}", "$${1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape02() {
        testEvaluation("$#{1+1}", "$2");
        testEvaluation("$\\#{1+1}", "$#{1+1}");  // is actually "$\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape03() {
        testEvaluation("##{1+1}", "#2");
        testEvaluation("#\\#{1+1}", "##{1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    @test
    public void testEscape04() {
        testEvaluation("#${1+1}", "#2");
        testEvaluation("#\\${1+1}", "#${1+1}");  // is actually "#\2" with {javax,jakarta}.el < 3.0.4
    }

    void testEvaluation(String expression, String expectedValue) {
        javax.el.ELContext context = new javax.el.ELProcessor().getELManager().getELContext();
        Object evaluatedExpression = javax.el.ELManager.getExpressionFactory()
                .createValueExpression(context, expression, String.class)
                .getValue(context);
        assertThat(evaluatedExpression).isEqualTo(expectedValue);
    }
```

Signed-off-by: Manfred Hanke <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GHSL-2020-021 - Bypass input sanitization of EL expressions