From d1454016012d739a4b7ff80b87b67b9bbb6c752d Mon Sep 17 00:00:00 2001 From: satamas Date: Mon, 15 Jul 2019 10:51:49 +0300 Subject: [PATCH 1/3] Add ability to limit output size for zip unarchiver as a way of protection against zip bombs. --- .../plexus/archiver/AbstractUnArchiver.java | 48 +++--- .../archiver/zip/AbstractZipUnArchiver.java | 162 ++++++++++-------- 2 files changed, 109 insertions(+), 101 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java index 58e121bbd..a53c2d538 100644 --- a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java @@ -16,15 +16,7 @@ */ package org.codehaus.plexus.archiver; -import java.io.File; -import java.io.FileNotFoundException; -import java.io.FileOutputStream; -import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; +import org.apache.commons.io.input.BoundedInputStream; import org.codehaus.plexus.archiver.util.ArchiveEntryUtils; import org.codehaus.plexus.components.io.attributes.SymlinkUtils; import org.codehaus.plexus.components.io.filemappers.FileMapper; @@ -35,6 +27,11 @@ import org.codehaus.plexus.util.IOUtil; import org.codehaus.plexus.util.StringUtils; +import java.io.*; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; + // TODO there should really be constructors which take the source file. /** @@ -316,9 +313,17 @@ public void setIgnorePermissions( final boolean ignorePermissions ) this.ignorePermissions = ignorePermissions; } - protected void extractFile( final File srcF, final File dir, final InputStream compressedInputStream, - String entryName, final Date entryDate, final boolean isDirectory, - final Integer mode, String symlinkDestination, final FileMapper[] fileMappers ) + protected File extractFile(final File srcF, final File dir, final InputStream compressedInputStream, + String entryName, final Date entryDate, final boolean isDirectory, + final Integer mode, String symlinkDestination, final FileMapper[] fileMappers) + throws IOException, ArchiverException { + return extractFile(srcF, dir, compressedInputStream, Long.MAX_VALUE, entryName, entryDate, isDirectory, + mode, symlinkDestination, fileMappers); + } + + protected File extractFile(final File srcF, final File dir, final InputStream compressedInputStream, + Long remainingSpace, String entryName, final Date entryDate, final boolean isDirectory, + final Integer mode, String symlinkDestination, final FileMapper[] fileMappers ) throws IOException, ArchiverException { if ( fileMappers != null ) @@ -345,7 +350,7 @@ protected void extractFile( final File srcF, final File dir, final InputStream c { if ( !isOverwrite() && f.exists() && ( f.lastModified() >= entryDate.getTime() ) ) { - return; + return f; } // create intermediary directories - sometimes zip don't add them @@ -365,18 +370,8 @@ else if ( isDirectory ) } else { - OutputStream out = null; - try - { - out = new FileOutputStream( f ); - - IOUtil.copy( compressedInputStream, out ); - out.close(); - out = null; - } - finally - { - IOUtil.close( out ); + try (OutputStream out = new FileOutputStream(f)) { + IOUtil.copy(new BoundedInputStream(compressedInputStream, remainingSpace), out); } } @@ -386,10 +381,13 @@ else if ( isDirectory ) { ArchiveEntryUtils.chmod( f, mode ); } + + return f; } catch ( final FileNotFoundException ex ) { getLogger().warn( "Unable to expand to file " + f.getPath() ); + return null; } } diff --git a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java index 4e185e270..d8a9f5ff9 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java @@ -16,6 +16,15 @@ */ package org.codehaus.plexus.archiver.zip; +import org.apache.commons.compress.archivers.zip.UnicodePathExtraField; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.codehaus.plexus.archiver.AbstractUnArchiver; +import org.codehaus.plexus.archiver.ArchiverException; +import org.codehaus.plexus.components.io.resources.PlexusIoResource; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -23,15 +32,6 @@ import java.net.URL; import java.util.Date; import java.util.Enumeration; -import javax.annotation.Nonnull; -import org.apache.commons.compress.archivers.zip.UnicodePathExtraField; -import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; -import org.apache.commons.compress.archivers.zip.ZipFile; -import org.apache.commons.compress.utils.IOUtils; -import org.codehaus.plexus.archiver.AbstractUnArchiver; -import org.codehaus.plexus.archiver.ArchiverException; -import org.codehaus.plexus.components.io.filemappers.FileMapper; -import org.codehaus.plexus.components.io.resources.PlexusIoResource; /** * @author Emmanuel Venisse @@ -44,6 +44,9 @@ public abstract class AbstractZipUnArchiver private String encoding = "UTF8"; + @Nullable + private Long maxOutputSize; + public AbstractZipUnArchiver() { } @@ -68,6 +71,14 @@ public void setEncoding( String encoding ) this.encoding = encoding; } + /** + * Set produced output size limit as a way of protection against zip bombs + * @param maxOutputSize - max size of produced output in bytes + */ + public void setMaxOutputSize(Long maxOutputSize) { + this.maxOutputSize = maxOutputSize; + } + private static class ZipEntryFileInfo implements PlexusIoResource { @@ -160,78 +171,58 @@ public boolean isExisting() protected void execute() throws ArchiverException { - getLogger().debug( "Expanding: " + getSourceFile() + " into " + getDestDirectory() ); - org.apache.commons.compress.archivers.zip.ZipFile zf = null; - InputStream in = null; - try + try(ZipFile zipFile = new ZipFile( getSourceFile(), encoding, true )) { - zf = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding, true ); - final Enumeration e = zf.getEntriesInPhysicalOrder(); + getLogger().debug( "Expanding: " + getSourceFile() + " into " + getDestDirectory() ); + Long remainingSpace = maxOutputSize; + final Enumeration e = zipFile.getEntriesInPhysicalOrder(); while ( e.hasMoreElements() ) { final ZipArchiveEntry ze = (ZipArchiveEntry) e.nextElement(); - final ZipEntryFileInfo fileInfo = new ZipEntryFileInfo( zf, ze ); + final ZipEntryFileInfo fileInfo = new ZipEntryFileInfo( zipFile, ze ); if ( isSelected( fileInfo.getName(), fileInfo ) ) { - in = zf.getInputStream( ze ); - - extractFileIfIncluded( getSourceFile(), getDestDirectory(), in, fileInfo.getName(), - new Date( ze.getTime() ), ze.isDirectory(), - ze.getUnixMode() != 0 ? ze.getUnixMode() : null, - resolveSymlink( zf, ze ), getFileMappers() ); - - in.close(); - in = null; + try (InputStream in = zipFile.getInputStream(ze)) { + if (remainingSpace != null) + { + File file = extractFile(getSourceFile(), getDestDirectory(), in, remainingSpace, + fileInfo.getName(), new Date(ze.getTime()), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink(zipFile, ze), getFileMappers()); + if (file != null) + { + remainingSpace -= file.length(); + if (remainingSpace <= 0) + { + throw new ArchiverException("Maximum output size limit reached"); + } + } + } + else + { + extractFile(getSourceFile(), getDestDirectory(), in, fileInfo.getName(), + new Date(ze.getTime()), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink(zipFile, ze), getFileMappers()); + } + } } } - - zf.close(); - zf = null; - getLogger().debug( "expand complete" ); } catch ( final IOException ioe ) { throw new ArchiverException( "Error while expanding " + getSourceFile().getAbsolutePath(), ioe ); } - finally - { - IOUtils.closeQuietly( in ); - IOUtils.closeQuietly( zf ); - } - } - - private String resolveSymlink( ZipFile zf, ZipArchiveEntry ze ) - throws IOException - { - if ( ze.isUnixSymlink() ) - { - return zf.getUnixSymlink( ze ); - } - else - { - return null; - } - } - - private void extractFileIfIncluded( final File sourceFile, final File destDirectory, final InputStream inputStream, - final String name, final Date time, final boolean isDirectory, - final Integer mode, String symlinkDestination, final FileMapper[] fileMappers ) - throws IOException, ArchiverException - { - extractFile( sourceFile, destDirectory, inputStream, name, time, isDirectory, mode, symlinkDestination, fileMappers ); } @Override protected void execute( final String path, final File outputDirectory ) throws ArchiverException { - org.apache.commons.compress.archivers.zip.ZipFile zipFile = null; - InputStream in = null; - try + try(ZipFile zipFile = new ZipFile( getSourceFile(), encoding, true )) { - zipFile = new org.apache.commons.compress.archivers.zip.ZipFile( getSourceFile(), encoding, true ); - + Long remainingSpace = maxOutputSize; final Enumeration e = zipFile.getEntriesInPhysicalOrder(); while ( e.hasMoreElements() ) @@ -245,30 +236,49 @@ protected void execute( final String path, final File outputDirectory ) if ( ze.getName().startsWith( path ) ) { - in = zipFile.getInputStream( ze ); - - extractFileIfIncluded( getSourceFile(), outputDirectory, in, - ze.getName(), new Date( ze.getTime() ), ze.isDirectory(), - ze.getUnixMode() != 0 ? ze.getUnixMode() : null, - resolveSymlink( zipFile, ze ), getFileMappers() ); - - in.close(); - in = null; + try(InputStream in = zipFile.getInputStream( ze )) { + if (remainingSpace != null) + { + File file = extractFile(getSourceFile(), outputDirectory, in, remainingSpace, + ze.getName(), new Date(ze.getTime()), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink(zipFile, ze), getFileMappers()); + if (file != null) + { + remainingSpace -= file.length(); + if (remainingSpace <= 0) + { + throw new ArchiverException("Maximum output size limit reached"); + } + } + } + else + { + extractFile(getSourceFile(), outputDirectory, in, + ze.getName(), new Date(ze.getTime()), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink(zipFile, ze), getFileMappers()); + } + } } } - - zipFile.close(); - zipFile = null; } catch ( final IOException ioe ) { throw new ArchiverException( "Error while expanding " + getSourceFile().getAbsolutePath(), ioe ); } - finally + } + + private String resolveSymlink( ZipFile zf, ZipArchiveEntry ze ) + throws IOException + { + if ( ze.isUnixSymlink() ) + { + return zf.getUnixSymlink( ze ); + } + else { - IOUtils.closeQuietly( in ); - IOUtils.closeQuietly( zipFile ); + return null; } } - } From 54704a6781d53d7535dadee66ff3d66bb8435234 Mon Sep 17 00:00:00 2001 From: satamas Date: Mon, 15 Jul 2019 11:37:56 +0300 Subject: [PATCH 2/3] Add zip output size limit test --- .../archiver/zip/ZipUnArchiverTest.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java b/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java index ea46a5aae..fbb95371d 100644 --- a/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java +++ b/src/test/java/org/codehaus/plexus/archiver/zip/ZipUnArchiverTest.java @@ -214,6 +214,31 @@ public void testExtractingZipWithEntryOutsideDestDirThrowsException() assertTrue( ex.getMessage().startsWith( "Entry is outside of the target directory" ) ); } + public void testZipOutputSizeException() + throws Exception + { + Exception ex = null; + String s = "target/zip-size-tests"; + File testZip = new File( getBasedir(), "src/test/jars/test.zip" ); + File outputDirectory = new File( getBasedir(), s ); + + FileUtils.deleteDirectory( outputDirectory ); + + try + { + ZipUnArchiver zu = getZipUnArchiver( testZip ); + zu.setMaxOutputSize(10L); + zu.extract( "", outputDirectory ); + } + catch ( Exception e ) + { + ex = e; + } + + assertNotNull( ex ); + assertTrue( ex.getMessage().startsWith( "Maximum output size limit reached" ) ); + } + private ZipArchiver getZipArchiver() { try From 74b2b3af42e987a59798f8324021462cb7bd16f1 Mon Sep 17 00:00:00 2001 From: satamas Date: Mon, 15 Jul 2019 14:02:50 +0300 Subject: [PATCH 3/3] Refactor imports after review --- .../plexus/archiver/AbstractUnArchiver.java | 16 ++++++++++------ .../archiver/zip/AbstractZipUnArchiver.java | 17 ++++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java index a53c2d538..31315b431 100644 --- a/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/AbstractUnArchiver.java @@ -16,7 +16,16 @@ */ package org.codehaus.plexus.archiver; -import org.apache.commons.io.input.BoundedInputStream; +import java.io.File; +import java.io.FileNotFoundException; +import java.io.FileOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.util.ArrayList; +import java.util.Date; +import java.util.List; +import org.apache.commons.compress.utils.BoundedInputStream; import org.codehaus.plexus.archiver.util.ArchiveEntryUtils; import org.codehaus.plexus.components.io.attributes.SymlinkUtils; import org.codehaus.plexus.components.io.filemappers.FileMapper; @@ -27,11 +36,6 @@ import org.codehaus.plexus.util.IOUtil; import org.codehaus.plexus.util.StringUtils; -import java.io.*; -import java.util.ArrayList; -import java.util.Date; -import java.util.List; - // TODO there should really be constructors which take the source file. /** diff --git a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java index d8a9f5ff9..1f96ea8a9 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java @@ -16,15 +16,6 @@ */ package org.codehaus.plexus.archiver.zip; -import org.apache.commons.compress.archivers.zip.UnicodePathExtraField; -import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; -import org.apache.commons.compress.archivers.zip.ZipFile; -import org.codehaus.plexus.archiver.AbstractUnArchiver; -import org.codehaus.plexus.archiver.ArchiverException; -import org.codehaus.plexus.components.io.resources.PlexusIoResource; - -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import java.io.File; import java.io.IOException; import java.io.InputStream; @@ -32,6 +23,14 @@ import java.net.URL; import java.util.Date; import java.util.Enumeration; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; +import org.apache.commons.compress.archivers.zip.UnicodePathExtraField; +import org.apache.commons.compress.archivers.zip.ZipArchiveEntry; +import org.apache.commons.compress.archivers.zip.ZipFile; +import org.codehaus.plexus.archiver.AbstractUnArchiver; +import org.codehaus.plexus.archiver.ArchiverException; +import org.codehaus.plexus.components.io.resources.PlexusIoResource; /** * @author Emmanuel Venisse