On Mon, 8 Feb 2016, Peter Fokker wrote:
> 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
(Nice that we managed to get an affected file in base.)
> 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).
Yep.
> 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:
Looks pretty good, though the *start=='/' check is unnecessary as if it
isn't a slash it's not a viable split point anyway. I'd also like to
preserve the comment about /TMNSZ-len, being unstorable in ustar, so I'm
planning on committing the slightly tweaked version of your diff seen
below.
Thank you for investigating this and tracking down the underlying issue.
Nice job!
Philip Guenther
Index: tar.c
===================================================================
RCS file: /data/src/openbsd/src/bin/pax/tar.c,v
retrieving revision 1.58
diff -u -p -r1.58 tar.c
--- tar.c 17 Mar 2015 03:23:17 -0000 1.58
+++ tar.c 14 Feb 2016 01:43:32 -0000
@@ -1159,6 +1159,16 @@ name_split(char *name, int len)
* include the slash between the two parts that gets thrown away)
*/
start = name + len - TNMSZ - 1;
+
+ /*
+ * the prefix may not be empty, so skip the first character when
+ * trying to split a path of exactly TNMSZ+1 characters.
+ * NOTE: This means the ustar format can't store /str if
+ * str contains no slashes and the length of str == TNMSZ
+ */
+ if (start == name)
+ ++start;
+
while ((*start != '\0') && (*start != '/'))
++start;
@@ -1168,15 +1178,12 @@ name_split(char *name, int len)
*/
if (*start == '\0')
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.
+ * the split point isn't valid if it results in a prefix
+ * longer than TPFSZ
*/
- if ((len > TPFSZ) || (len == 0))
+ if ((start - name) > TPFSZ)
return(NULL);
/*
