From 62a5af1eb334c98e1c7f298eb2ff7e19a64d1da8 Mon Sep 17 00:00:00 2001 From: Sergey Patrikeev Date: Fri, 23 Aug 2019 11:46:04 +0300 Subject: [PATCH 1/3] Remove redundant call to 'extractFileIfIncluded'. --- .../archiver/zip/AbstractZipUnArchiver.java | 25 ++++++------------- 1 file changed, 8 insertions(+), 17 deletions(-) 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 f35c64666..0586c4eb7 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java @@ -29,7 +29,6 @@ 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.filemappers.FileMapper; import org.codehaus.plexus.components.io.resources.PlexusIoResource; /** @@ -171,10 +170,10 @@ protected void execute() { try ( InputStream 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() ); + extractFile( getSourceFile(), getDestDirectory(), in, fileInfo.getName(), + new Date( ze.getTime() ), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink( zf, ze ), getFileMappers() ); } } } @@ -199,14 +198,6 @@ private String resolveSymlink( ZipFile zf, ZipArchiveEntry ze ) } } - 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 @@ -228,10 +219,10 @@ protected void execute( final String path, final File outputDirectory ) { try ( InputStream 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() ); + extractFile( getSourceFile(), outputDirectory, in, + ze.getName(), new Date( ze.getTime() ), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink( zipFile, ze ), getFileMappers() ); } } } From 40f072b84682af41a421ebb4648a5d22f8e54b8d Mon Sep 17 00:00:00 2001 From: Sergey Patrikeev Date: Fri, 23 Aug 2019 11:48:38 +0300 Subject: [PATCH 2/3] Remove code duplication. --- .../archiver/zip/AbstractZipUnArchiver.java | 28 ++----------------- 1 file changed, 3 insertions(+), 25 deletions(-) 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 0586c4eb7..7543f8981 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java @@ -158,31 +158,7 @@ public boolean isExisting() protected void execute() throws ArchiverException { - getLogger().debug( "Expanding: " + getSourceFile() + " into " + getDestDirectory() ); - try ( ZipFile zf = new ZipFile( getSourceFile(), encoding, true ) ) - { - final Enumeration e = zf.getEntriesInPhysicalOrder(); - while ( e.hasMoreElements() ) - { - final ZipArchiveEntry ze = e.nextElement(); - final ZipEntryFileInfo fileInfo = new ZipEntryFileInfo( zf, ze ); - if ( isSelected( fileInfo.getName(), fileInfo ) ) - { - try ( InputStream in = zf.getInputStream( ze ) ) - { - extractFile( getSourceFile(), getDestDirectory(), in, fileInfo.getName(), - new Date( ze.getTime() ), ze.isDirectory(), - ze.getUnixMode() != 0 ? ze.getUnixMode() : null, - resolveSymlink( zf, ze ), getFileMappers() ); - } - } - } - getLogger().debug( "expand complete" ); - } - catch ( final IOException ioe ) - { - throw new ArchiverException( "Error while expanding " + getSourceFile().getAbsolutePath(), ioe ); - } + execute("", getDestDirectory()); } private String resolveSymlink( ZipFile zf, ZipArchiveEntry ze ) @@ -202,6 +178,7 @@ private String resolveSymlink( ZipFile zf, ZipArchiveEntry ze ) protected void execute( final String path, final File outputDirectory ) throws ArchiverException { + getLogger().debug( "Expanding: " + getSourceFile() + " into " + outputDirectory ); try ( ZipFile zipFile = new ZipFile( getSourceFile(), encoding, true ) ) { final Enumeration e = zipFile.getEntriesInPhysicalOrder(); @@ -226,6 +203,7 @@ protected void execute( final String path, final File outputDirectory ) } } } + getLogger().debug( "expand complete" ); } catch ( final IOException ioe ) { From bb22cd4ba27ed033b345c5f600b641337a40656f Mon Sep 17 00:00:00 2001 From: Sergey Patrikeev Date: Fri, 23 Aug 2019 11:50:30 +0300 Subject: [PATCH 3/3] Add ability to limit output size for zip unarchiver as a way of protection against zip bombs. --- .../archiver/zip/AbstractZipUnArchiver.java | 36 ++++++++++++++++--- .../archiver/zip/ZipUnArchiverTest.java | 25 +++++++++++++ 2 files changed, 57 insertions(+), 4 deletions(-) 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 7543f8981..6319bb755 100644 --- a/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java +++ b/src/main/java/org/codehaus/plexus/archiver/zip/AbstractZipUnArchiver.java @@ -29,6 +29,8 @@ import org.apache.commons.compress.archivers.zip.ZipFile; import org.codehaus.plexus.archiver.AbstractUnArchiver; import org.codehaus.plexus.archiver.ArchiverException; +import org.apache.commons.io.input.BoundedInputStream; +import org.apache.commons.io.input.CountingInputStream; import org.codehaus.plexus.components.io.resources.PlexusIoResource; /** @@ -42,6 +44,8 @@ public abstract class AbstractZipUnArchiver private String encoding = "UTF8"; + private long maxOutputSize = Long.MAX_VALUE; + public AbstractZipUnArchiver() { } @@ -66,6 +70,21 @@ public void setEncoding( String encoding ) this.encoding = encoding; } + /** + * Set maximum allowed size of the produced output. + * + * It may be used as a protection against zip bombs. + * + * @param maxOutputSize max size of the produced output, in bytes. Must be greater than 0 + * @throws IllegalArgumentException if specified output size is less or equal to 0 + */ + public void setMaxOutputSize( long maxOutputSize ) { + if ( maxOutputSize <= 0 ) { + throw new IllegalArgumentException( "Invalid max output size specified: " + maxOutputSize ); + } + this.maxOutputSize = maxOutputSize; + } + private static class ZipEntryFileInfo implements PlexusIoResource { @@ -181,6 +200,7 @@ protected void execute( final String path, final File outputDirectory ) getLogger().debug( "Expanding: " + getSourceFile() + " into " + outputDirectory ); try ( ZipFile zipFile = new ZipFile( getSourceFile(), encoding, true ) ) { + long remainingSpace = maxOutputSize; final Enumeration e = zipFile.getEntriesInPhysicalOrder(); while ( e.hasMoreElements() ) @@ -196,10 +216,18 @@ protected void execute( final String path, final File outputDirectory ) { try ( InputStream in = zipFile.getInputStream( ze ) ) { - extractFile( getSourceFile(), outputDirectory, in, - ze.getName(), new Date( ze.getTime() ), ze.isDirectory(), - ze.getUnixMode() != 0 ? ze.getUnixMode() : null, - resolveSymlink( zipFile, ze ), getFileMappers() ); + BoundedInputStream bis = new BoundedInputStream( in, remainingSpace + 1 ); + CountingInputStream cis = new CountingInputStream( bis ); + extractFile( getSourceFile(), outputDirectory, cis, + ze.getName(), new Date( ze.getTime() ), ze.isDirectory(), + ze.getUnixMode() != 0 ? ze.getUnixMode() : null, + resolveSymlink( zipFile, ze ), getFileMappers() ); + + remainingSpace -= cis.getByteCount(); + if ( remainingSpace < 0 ) + { + throw new ArchiverException("Maximum output size limit reached"); + } } } } 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