You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by bo...@apache.org on 2021/05/01 09:03:17 UTC
[commons-compress] branch master updated: COMPRESS-575 extract and
harden parsing of GNU stuct_sparse entries
This is an automated email from the ASF dual-hosted git repository.
bodewig pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-compress.git
The following commit(s) were added to refs/heads/master by this push:
new ef765af COMPRESS-575 extract and harden parsing of GNU stuct_sparse entries
ef765af is described below
commit ef765af60786caf58be3efcf1fe84620bf4f2872
Author: Stefan Bodewig <bo...@apache.org>
AuthorDate: Sat May 1 11:01:13 2021 +0200
COMPRESS-575 extract and harden parsing of GNU stuct_sparse entries
I believe the code is wrong and we should drop empty structs even if
their offset is non-0 but this makes tests fail in non-obvious ways,
will revisit it later.
---
.../compress/archivers/tar/TarArchiveEntry.java | 12 ++-------
.../archivers/tar/TarArchiveSparseEntry.java | 12 +--------
.../commons/compress/archivers/tar/TarUtils.java | 30 ++++++++++++++++++++++
3 files changed, 33 insertions(+), 21 deletions(-)
diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java
index 12da7bb..fdeb0de 100644
--- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveEntry.java
@@ -1577,16 +1577,8 @@ public class TarArchiveEntry implements ArchiveEntry, TarConstants, EntryStreamO
offset += OFFSETLEN_GNU;
offset += LONGNAMESLEN_GNU;
offset += PAD2LEN_GNU;
- sparseHeaders = new ArrayList<>();
- for (int i = 0; i < SPARSE_HEADERS_IN_OLDGNU_HEADER; i++) {
- final TarArchiveStructSparse sparseHeader = TarUtils.parseSparse(header,
- offset + i * (SPARSE_OFFSET_LEN + SPARSE_NUMBYTES_LEN));
-
- // some sparse headers are empty, we need to skip these sparse headers
- if(sparseHeader.getOffset() > 0 || sparseHeader.getNumbytes() > 0) {
- sparseHeaders.add(sparseHeader);
- }
- }
+ sparseHeaders =
+ new ArrayList<>(TarUtils.readSparseStructs(header, offset, SPARSE_HEADERS_IN_OLDGNU_HEADER));
offset += SPARSELEN_GNU;
isExtended = TarUtils.parseBoolean(header, offset);
offset += ISEXTENDEDLEN_GNU;
diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveSparseEntry.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveSparseEntry.java
index b738de1..6973194 100644
--- a/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveSparseEntry.java
+++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarArchiveSparseEntry.java
@@ -57,17 +57,7 @@ public class TarArchiveSparseEntry implements TarConstants {
*/
public TarArchiveSparseEntry(final byte[] headerBuf) throws IOException {
int offset = 0;
- sparseHeaders = new ArrayList<>();
- for(int i = 0; i < SPARSE_HEADERS_IN_EXTENSION_HEADER;i++) {
- final TarArchiveStructSparse sparseHeader = TarUtils.parseSparse(headerBuf,
- offset + i * (SPARSE_OFFSET_LEN + SPARSE_NUMBYTES_LEN));
-
- // some sparse headers are empty, we need to skip these sparse headers
- if(sparseHeader.getOffset() > 0 || sparseHeader.getNumbytes() > 0) {
- sparseHeaders.add(sparseHeader);
- }
- }
-
+ sparseHeaders = new ArrayList<>(TarUtils.readSparseStructs(headerBuf, 0, SPARSE_HEADERS_IN_EXTENSION_HEADER));
offset += SPARSELEN_GNU_SPARSE;
isExtended = TarUtils.parseBoolean(headerBuf, offset);
}
diff --git a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java
index ea4741a..fe6fe4d 100644
--- a/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java
+++ b/src/main/java/org/apache/commons/compress/archivers/tar/TarUtils.java
@@ -25,6 +25,7 @@ import java.math.BigInteger;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -327,6 +328,35 @@ public class TarUtils {
}
/**
+ * @since 1.21
+ */
+ static List<TarArchiveStructSparse> readSparseStructs(final byte[] buffer, final int offset, final int entries)
+ throws IOException {
+ final List<TarArchiveStructSparse> sparseHeaders = new ArrayList<>();
+ for (int i = 0; i < entries; i++) {
+ try {
+ final TarArchiveStructSparse sparseHeader =
+ parseSparse(buffer, offset + i * (SPARSE_OFFSET_LEN + SPARSE_NUMBYTES_LEN));
+
+ if (sparseHeader.getOffset() < 0) {
+ throw new IOException("Corrupted TAR archive, sparse entry with negative offset");
+ }
+ if (sparseHeader.getNumbytes() < 0) {
+ throw new IOException("Corrupted TAR archive, sparse entry with negative numbytes");
+ }
+ // some sparse headers are empty, we need to skip these sparse headers
+ if (sparseHeader.getOffset() > 0 || sparseHeader.getNumbytes() > 0) {
+ sparseHeaders.add(sparseHeader);
+ }
+ } catch (IllegalArgumentException ex) {
+ // thrown internally by parseOctalOrBinary
+ throw new IOException("Corrupted TAR archive, sparse entry is invalid", ex);
+ }
+ }
+ return Collections.unmodifiableList(sparseHeaders);
+ }
+
+ /**
* Copy a name into a buffer.
* Copies characters from the name into the buffer
* starting at the specified offset.