When parsing GNU long (link) names, it is not guaranteed that the first
NUL byte is within the size set in the header. This leads to an
ambiguity: either the archive is malformed (wrong size in header) or the
string does not have to end with a NUL byte. The current implementation
of GNU tar is arguably wrong: given an 'L' member of size 4 whose data
block contains "name-rest", GNU tar ignores the size field and extracts
the next member under the name "name-rest", instead of "name" (matching
the size) or erroring.

* src/list.c (read_header): End a long name at the size of its member.

Signed-off-by: Harmen Stoppels <[email protected]>
---

Hello,

I'm working on an extensive test suite for tar file parsing, which is
available here https://github.com/haampie/tar-minefield with an
associated website with test results here https://stoppels.ch/tar/. As
part of this, I noticed a minor bug in GNU tar where it does not deal
correctly with the two ways in which a long (link) name can be ended:
either with an explicit NUL byte, or by the size from the header. I've
labeled this case https://stoppels.ch/tar/N39.html and you can see that
GNU tar is the odd one out here.

My patch is one solution, another would be to make GNU tar more strict
and let it error on this ambiguous archive. It would also be good to
document whether a trailing NUL byte is required within `size` bytes, so
that other implementations have something normative to refer to.

For what it's worth, it might be worth browsing through the
https://stoppels.ch/tar/gnu-tar.html page which lists test results of
GNU tar on all tricky archives I have accumulated so far.

Best,

Harmen Stoppels

 src/list.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/list.c b/src/list.c
index 6d4dc7ae..5179bde6 100644
--- a/src/list.c
+++ b/src/list.c
@@ -516,6 +516,8 @@ read_header (union block **return_block, struct 
tar_stat_info *info,
                }
 
              *bp = '\0';
+             /* The name ends at the size of the member */
+             charptr (header_copy + 1)[info->stat.st_size] = '\0';
              info->skipped = true;
            }
          else if (header->header.typeflag == XHDTYPE
-- 
2.53.0



Reply via email to