CVE-2021-23792 (Java, published 2022-05-06T20:15Z)
Description: value: The package com.twelvemonkeys.imageio:imageio-metadata before 3.7.1 are vulnerable to XML External Entity (XXE) Injection due to an insecurely initialized XML parser for reading XMP Metadata. An attacker can exploit this vulnerability if they are able to supply a file (e.g. when an online profile picture is processed) with a malicious XMP segment. If the XMP metadata of the uploaded image is parsed, then the XXE vulnerability is triggered.
Severity: HIGH (CVSS v3 base score: 9.8)
Weakness: CWE-611 \u2014 Improper Restriction of XML External Entity Reference
The product processes an XML document that can contain XML entities with URIs that resolve to documents outside of the intended sphere of control, causing the product to embed incorrect documents into its output.
Repository: https://github.com/haraldk/TwelveMonkeys
Files changed:
imageio/imageio-metadata/src/main/java/com/twelvemonkeys/imageio/metadata/xmp/XMPReader.java
imageio/imageio-metadata/src/test/java/com/twelvemonkeys/imageio/metadata/xmp/XMPReaderTest.java
imageio/imageio-metadata/src/test/resources/xmp/xmp-jpeg-xxe.xml
Commit message: Avoid fetching external resources in XMPReader.


Vulnerable code:
import com.twelvemonkeys.imageio.metadata.Directory;
import com.twelvemonkeys.imageio.metadata.Entry;
import com.twelvemonkeys.imageio.metadata.MetadataReader;
import com.twelvemonkeys.imageio.util.IIOUtil;
import com.twelvemonkeys.lang.Validate;
import javax.imageio.IIOException;
import javax.imageio.stream.ImageInputStream;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;
import java.io.IOException;
import java.util.*;
        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
        factory.setNamespaceAware(true);

//            XMLSerializer serializer = new XMLSerializer(System.err, System.getProperty("file.encoding"));
//            serializer.serialize(document);

            throw new RuntimeException(e); // TODO: Or IOException?
        Map<String, List<Entry>> subdirs = new LinkedHashMap<String, List<Entry>>();
                    dir = new ArrayList<Entry>();
                    Map<String, List<Entry>> subsubdirs = new LinkedHashMap<String, List<Entry>>();
        List<Directory> entries = new ArrayList<Directory>(subdirs.size());
        List<Entry> entries = new ArrayList<Entry>();
                dir = new ArrayList<Entry>();
                Map<String, Object> alternatives = new LinkedHashMap<String, Object>();
                List<Object> seq = new ArrayList<Object>();
import com.twelvemonkeys.imageio.metadata.CompoundDirectory;
import com.twelvemonkeys.imageio.metadata.Directory;
import com.twelvemonkeys.imageio.metadata.Entry;
import com.twelvemonkeys.imageio.metadata.MetadataReaderAbstractTest;

import org.junit.Test;
import javax.imageio.ImageIO;
import javax.imageio.stream.ImageInputStream;
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;

Fixed code:
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

import javax.imageio.IIOException;
import javax.imageio.stream.ImageInputStream;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.parsers.ParserConfigurationException;

import com.twelvemonkeys.imageio.metadata.Directory;
import com.twelvemonkeys.imageio.metadata.Entry;
import com.twelvemonkeys.imageio.metadata.MetadataReader;
import com.twelvemonkeys.imageio.util.IIOUtil;
import com.twelvemonkeys.lang.Validate;
            DocumentBuilderFactory factory = createDocumentBuilderFactory();

            throw new RuntimeException(e);
    private DocumentBuilderFactory createDocumentBuilderFactory() throws ParserConfigurationException {
        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
        factory.setNamespaceAware(true);

        // Security: Disable XInclude & expanding entity references ("bombs"), not needed for XMP
        factory.setXIncludeAware(false);
        factory.setExpandEntityReferences(false);

        // Security: Enable "secure processing", to prevent DoS attacks
        factory.setAttribute(XMLConstants.FEATURE_SECURE_PROCESSING, true);

        // Security: Remove possibility to access external DTDs or Schema, not needed for XMP
        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
        factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_SCHEMA, "");

        // Security: Disable loading of external DTD and entities, not needed for XMP
        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);

        return factory;
    }

        Map<String, List<Entry>> subdirs = new LinkedHashMap<>();
                    dir = new ArrayList<>();
                    Map<String, List<Entry>> subsubdirs = new LinkedHashMap<>();
        List<Directory> entries = new ArrayList<>(subdirs.size());
        List<Entry> entries = new ArrayList<>();
                dir = new ArrayList<>();
                Map<String, Object> alternatives = new LinkedHashMap<>();
                List<Object> seq = new ArrayList<>();
import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import java.io.OutputStream;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketException;
import java.nio.charset.StandardCharsets;
import javax.imageio.ImageIO;
import javax.imageio.stream.ImageInputStream;

import org.junit.Test;

import com.twelvemonkeys.imageio.metadata.CompoundDirectory;
import com.twelvemonkeys.imageio.metadata.Directory;
import com.twelvemonkeys.imageio.metadata.Entry;
import com.twelvemonkeys.imageio.metadata.MetadataReaderAbstractTest;

    @Test(timeout = 1500L)
    public void testNoExternalRequest() throws Exception {
        // TODO: Use dynamic port?
        try (HTTPServer server = new HTTPServer(7777)) {
            try {
                createReader().read(getResourceAsIIS("/xmp/xmp-jpeg-xxe.xml"));
            } catch (IOException ioe) {
                if (ioe.getMessage().contains("501")) {
                    throw new AssertionError("Reading should not cause external requests", ioe);
                }

                // Any other exception is a bug (but might happen if the parser does not support certain features)
                throw ioe;
            }
        }
    }

    private static class HTTPServer implements AutoCloseable {
        private final ServerSocket server;
        private final Thread thread;

        HTTPServer(int port) throws IOException {
            server = new ServerSocket(port, 1);
            thread = new Thread(new Runnable() {
                @Override public void run() {
                    serve();
                }
            });
            thread.start();
        }

        private void serve() {
            try {
                Socket client = server.accept();

                // Get the input stream, don't care about the request
                try (InputStream inputStream = client.getInputStream()) {
                    while (inputStream.available() > 0) {
                        if (inputStream.read() == -1) {
                            break;
                        }
                    }

                    // Answer with 501, this will cause the client to throw IOException
                    try (OutputStream outputStream = client.getOutputStream()) {
                        outputStream.write("HTTP/1.0 501 Not Implemented\r\n\r\n".getBytes(StandardCharsets.UTF_8));
                    }
                }
            }
            catch (IOException e) {
                if (server.isClosed() && e instanceof SocketException) {
                    // Socket closed due to server close, all good
                    return;
                }

                throw new RuntimeException(e);
            }
        }

        @Override public void close() throws Exception {
            server.close();
            thread.join(); // It's advised against throwing InterruptedException here, but this is not production code...
        }
    }
<?xpacket begin='﻿' id='W5M0MpCehiHzreSzNTczkc9d'?><!DOCTYPE root [<!ENTITY % ext SYSTEM 'http://localhost:7777/xxx'> %ext;]>
<x:xmpmeta xmlns:x='adobe:ns:meta/' x:xmptk='Image::ExifTool 10.16'>
<rdf:RDF xmlns:rdf='http://www.w3.org/1999/02/22-rdf-syntax-ns#'>

 <rdf:Description rdf:about=''
  xmlns:xmpMM='http://ns.adobe.com/xap/1.0/mm/'>
  <xmpMM:InstanceID>xmp.iid:7EDC21BF-371B-4189-90AF-C83A54A6A190</xmpMM:InstanceID>
 </rdf:Description>
</rdf:RDF>
</x:xmpmeta>
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
                                                                                                    
<?xpacket end='w'?>