From c501c643f7d54b0f3959a788281913435144aa5b Mon Sep 17 00:00:00 2001 From: Roberto Larcher Date: Tue, 1 Sep 2015 16:51:03 +0200 Subject: [PATCH 1/6] NegativeArraySizeException patch This excepion is raised when the attachment size is a multiple of 8176 (the buffer size). InputStream read methos return -1 if there is no more data because the end of the stream has been reached A simple check if result != -1 is added refs: http://forum.sleuthkit.org/viewtopic.php?f=6&t=2563 http://docs.oracle.com/javase/7/docs/api/java/io/InputStream.html#read() --- .../autopsy/thunderbirdparser/PstParser.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java b/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java index af3c55bf8f..05f313fd7e 100755 --- a/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java +++ b/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java @@ -258,9 +258,14 @@ class PstParser { out.write(buffer); count = attachmentStream.read(buffer); } - byte[] endBuffer = new byte[count]; - System.arraycopy(buffer, 0, endBuffer, 0, count); - out.write(endBuffer); + if(count != -1){ + byte[] endBuffer = new byte[count]; + System.arraycopy(buffer, 0, endBuffer, 0, count); + out.write(endBuffer); + } + else { + System.err.println(attach.toString()); + } } } From 3c0a34d1b2721ebb2b1da12a2587249477fb690e Mon Sep 17 00:00:00 2001 From: Roberto Larcher Date: Fri, 4 Sep 2015 11:23:24 +0200 Subject: [PATCH 2/6] Further analysis The problem seems to be in java-libpst. Sometimes getFileInputStream() returns an InputStream, but at the first InputStream.read(buffer) the method returns -1. Further analysis is needed to isolate the problem in java-libpst. To patch this an IOException is thrown --- .../autopsy/thunderbirdparser/PstParser.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java b/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java index 05f313fd7e..350ee6df6a 100755 --- a/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java +++ b/thunderbirdparser/src/org/sleuthkit/autopsy/thunderbirdparser/PstParser.java @@ -254,18 +254,19 @@ class PstParser { int bufferSize = 8176; byte[] buffer = new byte[bufferSize]; int count = attachmentStream.read(buffer); + + if(count == -1) { + throw new IOException("attachmentStream invalid (read() fails). File "+attach.getLongFilename()+ " skipped"); + } + while (count == bufferSize) { out.write(buffer); count = attachmentStream.read(buffer); } - if(count != -1){ - byte[] endBuffer = new byte[count]; - System.arraycopy(buffer, 0, endBuffer, 0, count); - out.write(endBuffer); - } - else { - System.err.println(attach.toString()); - } + + byte[] endBuffer = new byte[count]; + System.arraycopy(buffer, 0, endBuffer, 0, count); + out.write(endBuffer); } } From 813994e0f3b0f95814c682e2e34815ab1cf0de78 Mon Sep 17 00:00:00 2001 From: Richard Cordovano Date: Fri, 4 Sep 2015 11:19:35 -0400 Subject: [PATCH 3/6] Revert "turn on IG" --- ImageGallery/nbproject/project.xml | 2 +- ImageGallery/nbproject/suite.properties | 1 - nbproject/project.properties | 4 +--- 3 files changed, 2 insertions(+), 5 deletions(-) delete mode 100644 ImageGallery/nbproject/suite.properties diff --git a/ImageGallery/nbproject/project.xml b/ImageGallery/nbproject/project.xml index 2a28ce3deb..ab6cd75280 100644 --- a/ImageGallery/nbproject/project.xml +++ b/ImageGallery/nbproject/project.xml @@ -4,7 +4,7 @@ org.sleuthkit.autopsy.imagegallery - + org.netbeans.api.progress diff --git a/ImageGallery/nbproject/suite.properties b/ImageGallery/nbproject/suite.properties deleted file mode 100644 index 29d7cc9bd6..0000000000 --- a/ImageGallery/nbproject/suite.properties +++ /dev/null @@ -1 +0,0 @@ -suite.dir=${basedir}/.. diff --git a/nbproject/project.properties b/nbproject/project.properties index 6f01520695..d46c65d75a 100644 --- a/nbproject/project.properties +++ b/nbproject/project.properties @@ -27,13 +27,11 @@ modules=\ ${project.org.sleuthkit.autopsy.testing}:\ ${project.org.sleuthkit.autopsy.thunderbirdparser}:\ ${project.org.sleuthkit.autopsy.core}:\ - ${project.org.sleuthkit.autopsy.corelibs}:\ - ${project.org.sleuthkit.autopsy.imagegallery} + ${project.org.sleuthkit.autopsy.corelibs} project.org.sleuthkit.autopsy.core=Core project.org.sleuthkit.autopsy.corelibs=CoreLibs project.org.sleuthkit.autopsy.keywordsearch=KeywordSearch project.org.sleuthkit.autopsy.recentactivity=RecentActivity project.org.sleuthkit.autopsy.testing=Testing project.org.sleuthkit.autopsy.thunderbirdparser=thunderbirdparser -project.org.sleuthkit.autopsy.imagegallery=ImageGallery From 1b413e0ea12f7d3521710ec447fd00a113bd843e Mon Sep 17 00:00:00 2001 From: Brian Carrier Date: Tue, 8 Sep 2015 09:22:21 -0400 Subject: [PATCH 4/6] Sort methods in API docs --- docs/doxygen/Doxyfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/doxygen/Doxyfile b/docs/doxygen/Doxyfile index f3699e5c0e..e8701f1831 100755 --- a/docs/doxygen/Doxyfile +++ b/docs/doxygen/Doxyfile @@ -477,7 +477,7 @@ SORT_MEMBER_DOCS = YES # by member name. If set to NO (the default) the members will appear in # declaration order. -SORT_BRIEF_DOCS = NO +SORT_BRIEF_DOCS = YES # If the SORT_MEMBERS_CTORS_1ST tag is set to YES then doxygen # will sort the (brief and detailed) documentation of class members so that @@ -487,13 +487,13 @@ SORT_BRIEF_DOCS = NO # This tag will be ignored for brief docs if SORT_BRIEF_DOCS is set to NO # and ignored for detailed docs if SORT_MEMBER_DOCS is set to NO. -SORT_MEMBERS_CTORS_1ST = NO +SORT_MEMBERS_CTORS_1ST = YES # If the SORT_GROUP_NAMES tag is set to YES then doxygen will sort the # hierarchy of group names into alphabetical order. If set to NO (the default) # the group names will appear in their defined order. -SORT_GROUP_NAMES = NO +SORT_GROUP_NAMES = YES # If the SORT_BY_SCOPE_NAME tag is set to YES, the class list will be # sorted by fully-qualified names, including namespaces. If set to From 6f19fc7726253de06b536096fb228cb4c1e4cda8 Mon Sep 17 00:00:00 2001 From: momo Date: Tue, 8 Sep 2015 14:39:40 -0400 Subject: [PATCH 5/6] adding different child classes for each size cases --- .../SevenZipExtractor.java | 161 ++++++++++++------ 1 file changed, 109 insertions(+), 52 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java b/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java index b0934c1216..f5134f9cb8 100755 --- a/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java +++ b/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java @@ -146,7 +146,7 @@ class SevenZipExtractor { * @param abstractFile The AbstractFilw whose mimetype is to be determined. * * @return This method returns true if the file format is currently - * supported. Else it returns false. + * supported. Else it returns false. */ boolean isSevenZipExtractionSupported(AbstractFile abstractFile) { try { @@ -180,7 +180,7 @@ class SevenZipExtractor { * * More heuristics to be added here * - * @param archiveName the parent archive + * @param archiveName the parent archive * @param archiveFileItem the archive item * * @return true if potential zip bomb, false otherwise @@ -279,7 +279,7 @@ class SevenZipExtractor { * Unpack the file to local folder and return a list of derived files * * @param pipelineContext current ingest context - * @param archiveFile file to unpack + * @param archiveFile file to unpack * * @return list of unpacked derived files */ @@ -500,9 +500,9 @@ class SevenZipExtractor { //unpack locally if a file SevenZipExtractor.UnpackStream unpackStream = null; - if (!isDir) { + if (!isDir && size != null) { try { - unpackStream = new SevenZipExtractor.UnpackStream(localAbsPath, freeDiskSpace, size == null); + unpackStream = new SevenZipExtractor.KnownSizeUnpack(localAbsPath, size); item.extractSlow(unpackStream); } catch (Exception e) { //could be something unexpected with this file, move on @@ -510,22 +510,29 @@ class SevenZipExtractor { } finally { if (unpackStream != null) { //record derived data in unode, to be traversed later after unpacking the archive - if (size != null) { - // unpackedNode.bytesWritten will not be set in - // this case. Use 'size' which has been set - // previously. - unpackedNode.addDerivedInfo(size, !isDir, - 0L, createtime, accesstime, modtime, localRelPath); - } else { - // since size is unknown, use - // unpackStream.getNumberOfBytesWritten() to get - // the size. - unpackedNode.addDerivedInfo(unpackStream.getNumberOfBytesWritten(), !isDir, - 0L, createtime, accesstime, modtime, localRelPath); - } + unpackedNode.addDerivedInfo(unpackStream.getSize(), !isDir, + 0L, createtime, accesstime, modtime, localRelPath); unpackStream.close(); } } + } else if (!isDir && size == null) { + try { + unpackStream = new SevenZipExtractor.UnknownSizeUnpack(localAbsPath, freeDiskSpace); + item.extractSlow(unpackStream); + } catch (Exception e) { + //could be something unexpected with this file, move on + logger.log(Level.WARNING, "Could not extract file from archive: " + localAbsPath, e); //NON-NLS + } finally { + if (unpackStream != null) { + //record derived data in unode, to be traversed later after unpacking the archive + unpackedNode.addDerivedInfo(unpackStream.getSize(), !isDir, + 0L, createtime, accesstime, modtime, localRelPath); + unpackStream.close(); + } + } + } else { // this is a directory, size is always 0 + unpackedNode.addDerivedInfo(0, !isDir, + 0L, createtime, accesstime, modtime, localRelPath); } //update units for progress bar @@ -615,18 +622,12 @@ class SevenZipExtractor { /** * Stream used to unpack the archive to local file */ - private static class UnpackStream implements ISequentialOutStream { + private abstract static class UnpackStream implements ISequentialOutStream { - private OutputStream output; - private String localAbsPath; - private long freeDiskSpace; - private boolean sizeUnknown = false; - private boolean outOfSpace = false; - private long bytesWritten = 0; + protected OutputStream output; + protected String localAbsPath; - UnpackStream(String localAbsPath, long freeDiskSpace, boolean sizeUnknown) { - this.sizeUnknown = sizeUnknown; - this.freeDiskSpace = freeDiskSpace; + UnpackStream(String localAbsPath) { this.localAbsPath = localAbsPath; try { output = new BufferedOutputStream(new FileOutputStream(localAbsPath)); @@ -636,34 +637,59 @@ class SevenZipExtractor { } - public long getNumberOfBytesWritten() { + public abstract long getSize(); + + public void close() { + if (output != null) { + try { + output.flush(); + output.close(); + } catch (IOException e) { + logger.log(Level.SEVERE, "Error closing unpack stream for file: {0}", localAbsPath); //NON-NLS + } + } + } + } + + /** + * Stream used to unpack the archive of unknown size to local file + */ + private static class UnknownSizeUnpack extends UnpackStream { + + private long freeDiskSpace; + private boolean outOfSpace = false; + private long bytesWritten = 0; + + UnknownSizeUnpack(String localAbsPath, long freeDiskSpace) { + super(localAbsPath); + this.freeDiskSpace = freeDiskSpace; + } + + @Override + public long getSize() { return this.bytesWritten; } @Override public int write(byte[] bytes) throws SevenZipException { try { - if (!sizeUnknown) { + // If the content size is unknown, cautiously write to disk. + // Write only if byte array is less than 80% of the current + // free disk space. + if (freeDiskSpace == IngestMonitor.DISK_FREE_SPACE_UNKNOWN || bytes.length < 0.8 * freeDiskSpace) { output.write(bytes); + // NOTE: this method is called multiple times for a + // single extractSlow() call. Update bytesWritten and + // freeDiskSpace after every write operation. + this.bytesWritten += bytes.length; + this.freeDiskSpace -= bytes.length; } else { - // If the content size is unknown, cautiously write to disk. - // Write only if byte array is less than 80% of the current - // free disk space. - if (freeDiskSpace == IngestMonitor.DISK_FREE_SPACE_UNKNOWN || bytes.length < 0.8 * freeDiskSpace) { - output.write(bytes); - // NOTE: this method is called multiple times for a - // single extractSlow() call. Update bytesWritten and - // freeDiskSpace after every write operation. - this.bytesWritten += bytes.length; - this.freeDiskSpace -= bytes.length; - } else { - this.outOfSpace = true; - logger.log(Level.INFO, NbBundle.getMessage( - SevenZipExtractor.class, - "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.noSpace.msg")); - throw new SevenZipException( - NbBundle.getMessage(SevenZipExtractor.class, "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.noSpace.msg")); - } + this.outOfSpace = true; + logger.log(Level.INFO, NbBundle.getMessage( + SevenZipExtractor.class, + "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.noSpace.msg")); + throw new SevenZipException( + NbBundle.getMessage(SevenZipExtractor.class, "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.noSpace.msg")); } } catch (IOException ex) { throw new SevenZipException( @@ -673,6 +699,7 @@ class SevenZipExtractor { return bytes.length; } + @Override public void close() { if (output != null) { try { @@ -688,6 +715,36 @@ class SevenZipExtractor { } } + /** + * Stream used to unpack the archive of known size to local file + */ + private static class KnownSizeUnpack extends UnpackStream { + + private long size; + + KnownSizeUnpack(String localAbsPath, long size) { + super(localAbsPath); + this.size = size; + } + + @Override + public long getSize() { + return this.size; + } + + @Override + public int write(byte[] bytes) throws SevenZipException { + try { + output.write(bytes); + } catch (IOException ex) { + throw new SevenZipException( + NbBundle.getMessage(SevenZipExtractor.class, "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.exception.msg", + localAbsPath), ex); + } + return bytes.length; + } + } + /** * Representation of the files in the archive. Used to track of local tree * file hierarchy, archive depth, and files created to easily and reliably @@ -702,8 +759,8 @@ class SevenZipExtractor { /** * * @param localPathRoot Path in module output folder that files will be - * saved to - * @param archiveFile Archive file being extracted + * saved to + * @param archiveFile Archive file being extracted * @param fileManager */ UnpackedTree(String localPathRoot, AbstractFile archiveFile) { @@ -962,7 +1019,7 @@ class SevenZipExtractor { /** * Add a new archive to track of depth * - * @param parent parent archive or null + * @param parent parent archive or null * @param objectId object id of the new archive * * @return the archive added @@ -1003,4 +1060,4 @@ class SevenZipExtractor { } } -} \ No newline at end of file +} From 871d0d30cc1d45019071ed8b99100c948d0c82de Mon Sep 17 00:00:00 2001 From: momo Date: Tue, 8 Sep 2015 16:49:25 -0400 Subject: [PATCH 6/6] refactor duplication and names --- .../SevenZipExtractor.java | 60 +++++++++---------- 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java b/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java index f5134f9cb8..6ffa1df07e 100755 --- a/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java +++ b/Core/src/org/sleuthkit/autopsy/modules/embeddedfileextractor/SevenZipExtractor.java @@ -500,24 +500,12 @@ class SevenZipExtractor { //unpack locally if a file SevenZipExtractor.UnpackStream unpackStream = null; - if (!isDir && size != null) { + if (!isDir) { try { - unpackStream = new SevenZipExtractor.KnownSizeUnpack(localAbsPath, size); - item.extractSlow(unpackStream); - } catch (Exception e) { - //could be something unexpected with this file, move on - logger.log(Level.WARNING, "Could not extract file from archive: " + localAbsPath, e); //NON-NLS - } finally { - if (unpackStream != null) { - //record derived data in unode, to be traversed later after unpacking the archive - unpackedNode.addDerivedInfo(unpackStream.getSize(), !isDir, - 0L, createtime, accesstime, modtime, localRelPath); - unpackStream.close(); - } - } - } else if (!isDir && size == null) { - try { - unpackStream = new SevenZipExtractor.UnknownSizeUnpack(localAbsPath, freeDiskSpace); + if (size != null) + unpackStream = new SevenZipExtractor.KnownSizeUnpackStream(localAbsPath, size); + else + unpackStream = new SevenZipExtractor.UnknownSizeUnpackStream(localAbsPath, freeDiskSpace); item.extractSlow(unpackStream); } catch (Exception e) { //could be something unexpected with this file, move on @@ -624,8 +612,8 @@ class SevenZipExtractor { */ private abstract static class UnpackStream implements ISequentialOutStream { - protected OutputStream output; - protected String localAbsPath; + private OutputStream output; + private String localAbsPath; UnpackStream(String localAbsPath) { this.localAbsPath = localAbsPath; @@ -638,6 +626,14 @@ class SevenZipExtractor { } public abstract long getSize(); + + OutputStream getOutput() { + return output; + } + + String getLocalAbsPath() { + return localAbsPath; + } public void close() { if (output != null) { @@ -654,13 +650,13 @@ class SevenZipExtractor { /** * Stream used to unpack the archive of unknown size to local file */ - private static class UnknownSizeUnpack extends UnpackStream { + private static class UnknownSizeUnpackStream extends UnpackStream { private long freeDiskSpace; private boolean outOfSpace = false; private long bytesWritten = 0; - UnknownSizeUnpack(String localAbsPath, long freeDiskSpace) { + UnknownSizeUnpackStream(String localAbsPath, long freeDiskSpace) { super(localAbsPath); this.freeDiskSpace = freeDiskSpace; } @@ -677,7 +673,7 @@ class SevenZipExtractor { // Write only if byte array is less than 80% of the current // free disk space. if (freeDiskSpace == IngestMonitor.DISK_FREE_SPACE_UNKNOWN || bytes.length < 0.8 * freeDiskSpace) { - output.write(bytes); + getOutput().write(bytes); // NOTE: this method is called multiple times for a // single extractSlow() call. Update bytesWritten and // freeDiskSpace after every write operation. @@ -694,22 +690,22 @@ class SevenZipExtractor { } catch (IOException ex) { throw new SevenZipException( NbBundle.getMessage(SevenZipExtractor.class, "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.exception.msg", - localAbsPath), ex); + getLocalAbsPath()), ex); } return bytes.length; } @Override public void close() { - if (output != null) { + if (getOutput() != null) { try { - output.flush(); - output.close(); + getOutput().flush(); + getOutput().close(); if (this.outOfSpace) { - Files.delete(Paths.get(this.localAbsPath)); + Files.delete(Paths.get(getLocalAbsPath())); } } catch (IOException e) { - logger.log(Level.SEVERE, "Error closing unpack stream for file: {0}", localAbsPath); //NON-NLS + logger.log(Level.SEVERE, "Error closing unpack stream for file: {0}", getLocalAbsPath()); //NON-NLS } } } @@ -718,11 +714,11 @@ class SevenZipExtractor { /** * Stream used to unpack the archive of known size to local file */ - private static class KnownSizeUnpack extends UnpackStream { + private static class KnownSizeUnpackStream extends UnpackStream { private long size; - KnownSizeUnpack(String localAbsPath, long size) { + KnownSizeUnpackStream(String localAbsPath, long size) { super(localAbsPath); this.size = size; } @@ -735,11 +731,11 @@ class SevenZipExtractor { @Override public int write(byte[] bytes) throws SevenZipException { try { - output.write(bytes); + getOutput().write(bytes); } catch (IOException ex) { throw new SevenZipException( NbBundle.getMessage(SevenZipExtractor.class, "EmbeddedFileExtractorIngestModule.ArchiveExtractor.UnpackStream.write.exception.msg", - localAbsPath), ex); + getLocalAbsPath()), ex); } return bytes.length; }