Hello [email protected],

Today I discovered a problem with pax(1), running on OpenBSD 5.8 (amd64).
Below you find
 - a description of the problem,
 - an instruction on how to reproduce, and
 - a suggestion for a patch.


DESCRIPTION

I tried to add a file to an archive in ustar-format using pax(1) and I got this 
error:

pax: File name too long for ustar
/usr/include/g++/ext/pb_ds/detail/cc_hash_table_map_/constructor_destructor_no_store_hash_fn_imps.hpp

The filename part is only 48 chars, well within the 100-char limit of the ustar 
name field.
The path part is 52 chars, well within the 155-char limit of the ustar prefix 
field. Taking
into account the extra slash between path and filename the total length is 
exactly 52 + 1 + 48
= 101 chars.

This particular pathname could and should easily be split into two pieces, eg. 
"/usr" (4
chars) and
"include/g++/ext/pb_ds/detail/cc_hash_table_map_/constructor_destructor_no_store_hash_fn_imps.hpp"
(96 chars) or "/usr/include/g++/ext/pb_ds/detail/cc_hash_table_map_" (52 chars) 
and
"constructor_destructor_no_store_hash_fn_imps.hpp" (48 chars), but instead I 
get the error
message.

After some experiments and also studying the code I discovered that this error 
only happens
with an absolute path of exactly 101 chars. The routine responsible for 
splitting the name
into two parts (name_split()) erroneously tries to split at the first (leading) 
slash. That
eventually yields a perfect 100-char long name (good) but an invalid empty 
prefix (not good).

Note: there is no problem with a relative path of 101 chars because in that 
case name_split()
does not try to split at the very first character of the name.


HOW TO REPRODUCE

I created this little script in Puffy's home directory (/home/puffy).
It generates a few files with filename lengths between 86 and 102 characters. 
Together with
the length of the path "/home/puffy" (11 chars) and the slash between path and 
filename (1
char), I can test with pathname lengths between 11 + 1 + 86 = 98 chars and 11 + 
1 + 102 = 114
chars.


#!/bin/sh
XO=.XXXXXXXXX.OOOOOOOOO
XO=$XO$XO$XO$XO$XO
for x in 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102; do
  filename=$(printf "file-%04d%s\n" $x $XO | cut -c1-$x)
  echo -n $filename > /home/puffy/$filename
  echo /home/puffy/$filename | /bin/pax -w >pax-old-$x.tar
done


The script creates separate ustar-files and the output to stderr is as follows 
(filenames
pruned for readability):

pax: File name too long for ustar /home/puffy/file-0089.[...snip...]
pax: File name too long for ustar /home/puffy/file-0101.[...snip...]
pax: File name too long for ustar /home/puffy/file-0102.[...snip...]

Note that file-0089,* yields an error, but not file-0088.* nor file-0090.*.

The other two errors are to be expected because filenames of more than 100 
chars won't fit in
the ustar name field because they cannot be split at a slash. However, 
file-0100.* and
file-0099.* and all the others are archived without a problem.


PROPOSED PATCH

I have patched src/bin/pax/tar.c and with the patched version adding 
file-0089.* to a ustar
archive no longer yields the error. The errors for file-0101.* and file-0102.* 
remain, which
is expected behaviour. Here is the diff:

--- src/bin/pax/tar.c-orig      Tue Mar 17 04:23:17 2015
+++ src/bin/pax/tar.c   Mon Feb  8 16:36:14 2016
@@ -1159,6 +1159,15 @@
         * include the slash between the two parts that gets thrown away)
         */
        start = name + len - TNMSZ - 1;
+
+       /*
+        * If name is an absolute path of exactly TNMSZ+1 characters we
+        * start looking for the second / (if any) instead of splitting
+        * at the initial / which yields an unacceptable empty prefix.
+        */
+       if ((start == name) && (*start == '/'))
+               ++start;
+
        while ((*start != '\0') && (*start != '/'))
                ++start;

@@ -1170,13 +1179,7 @@
                return(NULL);
        len = start - name;

-       /*
-        * NOTE: /str where the length of str == TNMSZ can not be stored under
-        * the p1003.1-1990 spec for ustar. We could force a prefix of / and
-        * the file would then expand on extract to //str. The len == 0 below
-        * makes this special case follow the spec to the letter.
-        */
-       if ((len > TPFSZ) || (len == 0))
+       if (len > TPFSZ)
                return(NULL);

        /*

I have also attached this diff, retaining the tab-characters in the code.
It would be very nice if you would consider this patch for a future release.
If you need more information I am more than happy to provide it.

Kind regards from The Netherlands,

--Peter Fokker

PS. Please, be gentle with me, this is my first patch to the OpenBSD project...

Attachment: src-bin-pax-tar.c-patch
Description: Binary data

Reply via email to