On Sun, Jun 29, 2025 at 09:02:27PM +0200, Jeremie Courreges-Anglas wrote:
> On Sat, Jun 28, 2025 at 09:02:16AM -0600, Todd C. Miller wrote:
> > On Sun, 22 Jun 2025 04:38:25 -0300, Crystal Kolipe wrote:
> > 
> > > It's possible for a non-root user with no special permissions to disrupt
> > > backups made by root using tar and pax.  Note that cpio is not affected.
> > >
> > > This is done by abusing file modification timestamps and fooling tar into
> > > quitting early.
> > 
> > The underlying issue is that pax was silently failing when writing
> > out the extended headers if the mtime didn't fit in the ustar header
> > for the extended header record.  This should not be a fatal error
> > since the extended header is not a real file and will only be
> > extracted by versions of tar that doesn't support pax format.
> 
> Indeed, that wasn't very good error checking. :-/
> I'll cook another diff to improve the other error cases.

Print a message when failing to write tar, ustar and pax (extended)
headers.

Do no not abort but skip the file if the extended header size or
checksum do not fit, same as for regular ustar headers.  Not sure if
it can really happen in practice.

ok?


Index: tar.c
===================================================================
RCS file: /cvs/src/bin/pax/tar.c,v
diff -u -p -r1.86 tar.c
--- tar.c	29 Jun 2025 20:29:48 -0000	1.86
+++ tar.c	1 Jul 2025 16:11:01 -0000
@@ -658,10 +658,11 @@ tar_wr(ARCHD *arcn)
 	if (ul_oct(tar_chksm(hdblk, sizeof(HD_TAR)), hd->chksum,
 	    sizeof(hd->chksum), 3))
 		goto out;
-	if (wr_rdbuf(hdblk, sizeof(HD_TAR)) < 0)
-		return(-1);
-	if (wr_skip(BLKMULT - sizeof(HD_TAR)) < 0)
+	if (wr_rdbuf(hdblk, sizeof(HD_TAR)) < 0 ||
+	    wr_skip(BLKMULT - sizeof(HD_TAR)) < 0) {
+		paxwarn(1,"Could not write tar header for %s", arcn->org_name);
 		return(-1);
+	}
 	if (PAX_IS_REG(arcn->type))
 		return(0);
 	return(1);
@@ -1055,7 +1056,7 @@ wr_xheader(char *fname, HD_USTAR *fhd, s
 	strncpy(hd->magic, TMAGIC, TMAGLEN);
 	strncpy(hd->version, TVERSION, TVERSLEN);
 	if (ul_oct(size, hd->size, sizeof(hd->size), 3))
-		return -1;
+		goto out;
 
 	/*
 	 * Best effort attempt at providing a useful file name for
@@ -1078,23 +1079,33 @@ wr_xheader(char *fname, HD_USTAR *fhd, s
 	memcpy(hd->gid, fhd->gid, sizeof(hd->gid));
 	if (ul_oct(tar_chksm(hdblk, sizeof(HD_USTAR)), hd->chksum,
 	   sizeof(hd->chksum), 3))
-		return -1;
+		goto out;
 
 	/* write out extended header */
-	if (wr_rdbuf(hdblk, sizeof(HD_USTAR)) < 0)
-		return -1;
-	if (wr_skip(BLKMULT - sizeof(HD_USTAR)) < 0)
-		return -1;
+	if (wr_rdbuf(hdblk, sizeof(HD_USTAR)) < 0 ||
+	    wr_skip(BLKMULT - sizeof(HD_USTAR)) < 0)
+		goto err;
 
 	/* write out extended header records */
 	SLIST_FOREACH(rec, xhdr, entry)
 		if (wr_rdbuf(rec->record, rec->reclen) < 0)
-			return -1;
+			goto err;
 
 	if (wr_skip(TAR_PAD(size)) < 0)
-		return -1;
+		goto err;
 
 	return 0;
+
+out:
+	/*
+	 * header field is out of range
+	 */
+	paxwarn(1, "Pax header field is too small for %s", fname);
+	return 1;
+
+err:
+	paxwarn(1,"Could not write pax extended header for %s", fname);
+	return -1;
 }
 #endif
 
@@ -1341,8 +1352,8 @@ wr_ustar_or_pax(ARCHD *arcn, int ustar)
 
 		ret = wr_xheader(arcn->name, hd, &xhdr);
 		xheader_free(&xhdr);
-		if (ret == -1)
-			return(-1);
+		if (ret)
+			return(ret);
 	}
 #endif
 
@@ -1354,10 +1365,12 @@ wr_ustar_or_pax(ARCHD *arcn, int ustar)
 	if (ul_oct(tar_chksm(hdblk, sizeof(HD_USTAR)), hd->chksum,
 	   sizeof(hd->chksum), 3))
 		goto out;
-	if (wr_rdbuf(hdblk, sizeof(HD_USTAR)) < 0)
-		return(-1);
-	if (wr_skip(BLKMULT - sizeof(HD_USTAR)) < 0)
+	if (wr_rdbuf(hdblk, sizeof(HD_USTAR)) < 0 ||
+	    wr_skip(BLKMULT - sizeof(HD_USTAR)) < 0) {
+		paxwarn(1,"Could not write ustar header for %s",
+		    arcn->org_name);
 		return(-1);
+	}
 	if (PAX_IS_REG(arcn->type))
 		return(0);
 	return(1);

-- 
jca