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

XXE vulnerability in GenericPackager #462

Open
k-sever opened this issue Jan 5, 2022 · 3 comments · May be fixed by #463
Open

XXE vulnerability in GenericPackager #462

k-sever opened this issue Jan 5, 2022 · 3 comments · May be fixed by #463

Comments

@k-sever
Copy link

k-sever commented Jan 5, 2022

GenericPackager (as well as GenericValidatingPackager) are vulnerable to XML External Entity (XEE) attack.

Here is the test example that reproduces it

@Test
    public void testXXEAttach() {
        try {
            String xeeAttackXml = "<?xml version=\"1.0\" encoding=\"ISO-8859-1\"?>\n"
                + "<!DOCTYPE foo [\n"
                + "  <!ELEMENT foo ANY >\n"
                + "  <!ENTITY xxe SYSTEM \"https://jposxee.free.beeceptor.com/hacked\" >]>\n"
                + "<foo>&xxe;</foo>";
            new GenericPackager().readFile(new ByteArrayInputStream(xeeAttackXml.getBytes()));
            fail("Expected ISOException to be thrown");
        } catch (ISOException ex) {
            assertEquals("Entity extension is disabled", ex.getMessage(), "ex.getMessage()");
        }
    }

The xxe entity is getting resolved and request goes to https://jposxee.free.beeceptor.com/hacked (it's just a mock API):
Screenshot 2022-01-05 at 22 34 45

It's not a finalized test that you can check in, but it gives you the idea.

@k-sever k-sever linked a pull request Jan 5, 2022 that will close this issue
@k-sever k-sever closed this as completed Jan 10, 2022
@ar
Copy link
Member

ar commented Jan 10, 2022

GenericPackager configuration is usually read from the classpath. Having the ability to define entities is a nice to have feature that improves code readability. Somebody with enough access to the classpath to use an XXE have easier ways to compromise the system.

Can you provide a use case where a GenericPackager configuration could be injected from a remote host?

@k-sever
Copy link
Author

k-sever commented Jan 13, 2022

GenericPackager configuration is usually read from the classpath.

Well, you can't enforce it. IMO it should be secure by default, security > code readability. I'd rather disable external entities by default and make it possible to configure XML reader features for GenericPackager, so that anyone who actually needs it can redefine it.

@ar
Copy link
Member

ar commented Jan 13, 2022

Fair enough. I'll reopen the issue and think about a way to configure it to reenable the feature.

@ar ar reopened this Jan 13, 2022
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 a pull request may close this issue.

2 participants