Mailing-List: contact issues-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: issues@commons.apache.org
Date: Tue, 15 Nov 2016 07:25:59 +0000 (UTC)
From: "Mark Thomas (JIRA)" <jira@apache.org>
To: issues@commons.apache.org
Message-ID: <JIRA.13020471.1479119627000.277939.1479194759964@Atlassian.JIRA>
In-Reply-To: <JIRA.13020471.1479119627000@Atlassian.JIRA>
References: <JIRA.13020471.1479119627000@Atlassian.JIRA> <JIRA.13020471.1479119627096@arcas>
Subject: [jira] [Commented] (FILEUPLOAD-279) CVE-2016-1000031 - Apache
 Commons FileUpload DiskFileItem File Manipulation Remote Code Execution
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
archived-at: Tue, 15 Nov 2016 07:26:02 -0000


    [ https://issues.apache.org/jira/browse/FILEUPLOAD-279?page=3Dcom.atlas=
sian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=3D=
15666372#comment-15666372 ]=20

Mark Thomas commented on FILEUPLOAD-279:
----------------------------------------

-1 to back-porting since it breaks backwards compatibility.

This was discussed at the time and the security hardening changes were appl=
ied only to trunk because of backwards compatibility concerns.

To re-iterate what has been said elsewhere. Java (de)serialization is inher=
ently dangerous. It should only be performed from trusted sources or after =
appropriate validation that only safe classes are being used. Applications =
that fail to do this are open to all sorts of vulnerabilities.

> CVE-2016-1000031 - Apache Commons FileUpload DiskFileItem File Manipulati=
on Remote Code Execution
> -------------------------------------------------------------------------=
------------------------
>
>                 Key: FILEUPLOAD-279
>                 URL: https://issues.apache.org/jira/browse/FILEUPLOAD-279
>             Project: Commons FileUpload
>          Issue Type: Bug
>    Affects Versions: 1.3.2
>            Reporter: Michiel Weggen
>            Priority: Critical
>              Labels: security
>         Attachments: fix2.patch
>
>
> http://www.tenable.com/security/research/tra-2016-12
> Summary
> There exists a Java Object in the Apache Commons FileUpload library that =
can be manipulated in such a way that when it is deserialized, it can write=
 or copy files to disk in arbitrary locations. Furthermore, while the Objec=
t can be used alone, this new vector can be integrated with ysoserial to up=
load and execute binaries in a single deserialization call. This may or may=
 not work depending on an application's implementation of the FileUpload li=
brary.
> Background
> In late 2015 FoxGlove Security released a write up on using Chris Frohoff=
=E2=80=99s yososerial tool to gain remote code execution on a variety of co=
mmercial products, based on a presentation at AppSec Cali in January, 2015.=
 The ysoserial tool uses =E2=80=9Cgadgets=E2=80=9D in Apache Commons Collec=
tions, Groovy, and Spring that do =E2=80=9Cunexpected=E2=80=9D things durin=
g deserialization. Specifically, the ysoserial payloads eventually execute =
Runtime.getRuntime().exec() allowing for remote Java code execution.
> The Apache Commons project maintains a library called =E2=80=9CFileUpload=
=E2=80=9D to make =E2=80=9Cit easy to add robust, high-performance, file up=
load capability to your servlets and web applications.=E2=80=9D One of the =
classes in the FileUpload library is called =E2=80=9CDiskFileItem=E2=80=9D.=
 A DiskFileItem is used to handle file uploads. Interestingly, DiskFileItem=
 is serializable and implements custom writeObject() and readObject() funct=
ions.
> DiskFileItem=E2=80=99s readObject Implementation
> Here is the implementation that currently exists at the projects reposito=
ry tip (as of 1/28/16):
> 632    private void readObject(ObjectInputStream in)
> 633            throws IOException, ClassNotFoundException {
> 634        // read values
> 635        in.defaultReadObject();
> 636
> 637        /* One expected use of serialization is to migrate HTTP sessio=
ns
> 638         * containing a DiskFileItem between JVMs. Particularly if the=
 JVMs are
> 639         * on different machines It is possible that the repository lo=
cation is
> 640         * not valid so validate it.
> 641         */
> 642        if (repository !=3D null) {
> 643            if (repository.isDirectory()) {
> 644                // Check path for nulls
> 645                if (repository.getPath().contains("\0")) {
> 646                    throw new IOException(format(
> 647                            "The repository [%s] contains a null chara=
cter",
> 648                            repository.getPath()));
> 649                }
> 650            } else {
> 651                throw new IOException(format(
> 652                        "The repository [%s] is not a directory",
> 653                        repository.getAbsolutePath()));
> 654            }
> 655        }
> 656
> 657        OutputStream output =3D getOutputStream();
> 658        if (cachedContent !=3D null) {
> 659            output.write(cachedContent);
> 660        } else {
> 661            FileInputStream input =3D new FileInputStream(dfosFile);
> 662            IOUtils.copy(input, output);
> 663            IOUtils.closeQuietly(input);
> 664            dfosFile.delete();
> 665            dfosFile =3D null;
> 666        }
> 667        output.close();
> 668
> 669        cachedContent =3D null;
> 670    }
> This is interesting due to the apparent creation of files. However, after=
 analyzing the state of DiskFileItem after serialization it became clear th=
at arbitrary file creation was not supposed to be intended:
> dfos (a type of OutputStream) is transient and therefore it is not serial=
ized. dfos is regenerated by the getOutputStream() call above (which also g=
enerates the new File to write out to).
> The =E2=80=9Crepository=E2=80=9D (or directory that the file is written t=
o) has to be valid at the time of serialization in order for successful des=
erialization to occur.
> If there is no =E2=80=9CcachedContent=E2=80=9D then readObject() tries to=
 read in the file from disk.
> That filename is always generated via getOutputStream.
> Serialized Object Modification
> The rules listed above do not take into account that someone might modify=
 the serialized data before it is deserialized. Three important elements ge=
t serialized that we can modify:
> The repository path (aka the directory that the file is read/written from=
).
> If there is cachedContent (i.e. data that didn=E2=80=99t get written to t=
he file) then that gets serialized
> If there is no cachedContent (i.e. all data was written to disk) the full=
 path to the output file exists.
> The threshold value that controls if =E2=80=9CcachedContent=E2=80=9D is w=
ritten to disk or not.
> Modifying these three elements in the serialized object gives us the abil=
ity to:
> Create files wherever we have permission on the system. The caveat here i=
s that we only have control of the file path and not the final filename.
> Copy the contents of files from one file on the system to a location we s=
pecify (again we only control the directory path and not the filename). Thi=
s will also attempt to delete the file we copy from.. so be careful.


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)