From d415592b376016f9b18314aff7cfbcf99dfe7e64 Mon Sep 17 00:00:00 2001
From: tg <tg@mirbsd.org>
Date: Sun, 6 Jul 2008 22:41:09 +0000
Subject: [PATCH] check return value of unlink(2) when trying to remove an
 existing HISTFILE, since mksh(1) did go into an infinite loop if that fails
 first
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

bug spotted, initial patch and help drafting a test case
From: Decklin Foster <decklin@red-bean.com>

note there are more instances of unlink(2) and others (like chmod(2), as
spotted by flawfinder) which aren’t checked… but at least the other case
of unlink(2) use in histrap.c doesn’t cause any trouble (I think)
---
 check.t   | 26 ++++++++++++++++++++++++--
 histrap.c | 16 +++++++++++++---
 sh.h      |  4 ++--
 3 files changed, 39 insertions(+), 7 deletions(-)

diff --git a/check.t b/check.t
index 05a2c9e..1ab1ec9 100644
--- a/check.t
+++ b/check.t
@@ -1,4 +1,4 @@
-# $MirOS: src/bin/mksh/check.t,v 1.201 2008/06/28 22:51:54 tg Exp $
+# $MirOS: src/bin/mksh/check.t,v 1.202 2008/07/06 22:41:07 tg Exp $
 # $OpenBSD: bksl-nl.t,v 1.2 2001/01/28 23:04:56 niklas Exp $
 # $OpenBSD: history.t,v 1.5 2001/01/28 23:04:56 niklas Exp $
 # $OpenBSD: read.t,v 1.3 2003/03/10 03:48:16 david Exp $
@@ -7,7 +7,7 @@
 # http://www.research.att.com/~gsf/public/ifs.sh
 
 expected-stdout:
-	@(#)MIRBSD KSH R34 2008/06/28
+	@(#)MIRBSD KSH R34 2008/07/06
 description:
 	Check version of shell.
 stdin:
@@ -1718,6 +1718,28 @@ expected-stdout:
 expected-stderr-pattern:
 	/^X*$/
 ---
+name: history-unlink
+description:
+	Check if broken HISTFILEs do not cause trouble
+arguments: !-i!
+env-setup: !ENV=./Env!HISTFILE=foo/hist.file!
+file-setup: file 644 "Env"
+	PS1=X
+file-setup: dir 755 "foo"
+file-setup: file 644 "foo/hist.file"
+	sometext
+time-limit: 5
+perl-setup: chmod(0555, "foo");
+stdin:
+	echo hi
+	fc -l
+	chmod 0755 foo
+expected-stdout:
+	hi
+	1	echo hi
+expected-stderr-pattern:
+	/.*cannot unlink HISTFILE.*\nX*$/
+---
 name: history-e-minus-1
 description:
 	Check if more recent command is executed
diff --git a/histrap.c b/histrap.c
index 905893f..bc6add3 100644
--- a/histrap.c
+++ b/histrap.c
@@ -3,7 +3,7 @@
 
 #include "sh.h"
 
-__RCSID("$MirOS: src/bin/mksh/histrap.c,v 1.64 2008/06/08 17:16:25 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/histrap.c,v 1.65 2008/07/06 22:41:08 tg Exp $");
 
 /*-
  * MirOS: This is the default mapping type, and need not be specified.
@@ -677,17 +677,27 @@ hist_init(Source *s)
 			if (base != (unsigned char *)MAP_FAILED)
 				munmap((caddr_t)base, hsize);
 			hist_finish();
-			unlink(hname);
+			if (unlink(hname) /* fails */)
+				goto hiniterr;
 			goto retry;
 		}
 		if (hsize > 2) {
+			int rv = 0;
+
 			lines = hist_count_lines(base+2, hsize-2);
 			if (lines > histsize) {
 				/* we need to make the file smaller */
 				if (hist_shrink(base, hsize))
-					unlink(hname);
+					rv = unlink(hname);
 				munmap((caddr_t)base, hsize);
 				hist_finish();
+				if (rv) {
+ hiniterr:
+					bi_errorf("cannot unlink HISTFILE %s"
+					    " - %s", hname, strerror(errno));
+					hsize = 0;
+					return;
+				}
 				goto retry;
 			}
 		}
diff --git a/sh.h b/sh.h
index 689065a..5028077 100644
--- a/sh.h
+++ b/sh.h
@@ -100,9 +100,9 @@
 #define __SCCSID(x)	__IDSTRING(sccsid,x)
 
 #ifdef EXTERN
-__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.220 2008/06/28 22:51:55 tg Exp $");
+__RCSID("$MirOS: src/bin/mksh/sh.h,v 1.221 2008/07/06 22:41:09 tg Exp $");
 #endif
-#define MKSH_VERSION "R34 2008/06/28"
+#define MKSH_VERSION "R34 2008/07/06"
 
 #ifndef MKSH_INCLUDES_ONLY