If a .shift request has a negative argument, do not use a negative array

index but use 0 instead of the argument, just like groff.
Warn about the invalid argument.
While here, fix the column number in another warning message.

Segfault reported by tb@, found with afl(1).

7 files changed, 36 insertions, 15 deletions

diff --git a/mandoc.1 b/mandoc.1
index 176ccf9c..415769b3 100644
--- a/mandoc.1
+++ b/mandoc.1
@@ -1,6 +1,6 @@
-.\" $Id: mandoc.1,v 1.256 2022/04/14 10:10:22 schwarze Exp $
+.\" $Id: mandoc.1,v 1.257 2022/04/24 13:38:46 schwarze Exp $
 .\"
-.\" Copyright (c) 2012, 2014-2021 Ingo Schwarze <schwarze@openbsd.org>
+.\" Copyright (c) 2012, 2014-2022 Ingo Schwarze <schwarze@openbsd.org>
 .\" Copyright (c) 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
 .\"
 .\" Permission to use, copy, modify, and distribute this software for any
@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd $Mdocdate: April 14 2022 $
+.Dd $Mdocdate: April 24 2022 $
 .Dt MANDOC 1
 .Os
 .Sh NAME
@@ -2082,6 +2082,13 @@ and expands to the empty string.
 .Pq roff
 The argument of the escape sequence \e$ is not a digit;
 the escape sequence expands to the empty string.
+.It Sy "negative argument, using 0"
+.Pq roff
+A
+.Ic \&shift
+request has a negative argument
+or an argument that is negative due to integer overflow.
+Macro argument numbering remains unchanged.
 .It Sy "NOT IMPLEMENTED: Bd -file"
 .Pq mdoc
 For security reasons, the
diff --git a/mandoc.h b/mandoc.h
index 9837ff2a..32914deb 100644
--- a/mandoc.h
+++ b/mandoc.h
@@ -1,6 +1,6 @@
-/* $Id: mandoc.h,v 1.274 2021/08/14 13:53:08 schwarze Exp $ */
+/* $Id: mandoc.h,v 1.275 2022/04/24 13:38:46 schwarze Exp $ */
 /*
- * Copyright (c) 2012-2021 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2012-2022 Ingo Schwarze <schwarze@openbsd.org>
  * Copyright (c) 2010, 2011, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -215,6 +215,7 @@ enum	mandocerr {
 	MANDOCERR_NAMESC, /* escaped character not allowed in a name: name */
 	MANDOCERR_ARG_UNDEF, /* using macro argument outside macro */
 	MANDOCERR_ARG_NONUM, /* argument number is not numeric */
+	MANDOCERR_ARG_NEG, /* negative argument, using 0: request arg */
 	MANDOCERR_BD_FILE, /* NOT IMPLEMENTED: Bd -file */
 	MANDOCERR_BD_NOARG, /* skipping display without arguments: Bd */
 	MANDOCERR_BL_NOTYPE, /* missing list type, using -item: Bl */
diff --git a/mandoc_msg.c b/mandoc_msg.c
index beec5059..0361fc80 100644
--- a/mandoc_msg.c
+++ b/mandoc_msg.c
@@ -1,6 +1,6 @@
 /* $OpenBSD: mandoc_msg.c,v 1.8 2020/01/19 17:59:01 schwarze Exp $ */
 /*
- * Copyright (c) 2014-2021 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2014-2022 Ingo Schwarze <schwarze@openbsd.org>
  * Copyright (c) 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -216,6 +216,7 @@ static	const char *const type_message[MANDOCERR_MAX] = {
 	"escaped character not allowed in a name",
 	"using macro argument outside macro",
 	"argument number is not numeric",
+	"negative argument, using 0",
 	"NOT IMPLEMENTED: Bd -file",
 	"skipping display without arguments",
 	"missing list type, using -item",
diff --git a/regress/roff/shift/bad.in b/regress/roff/shift/bad.in
index 809832de..22eff7e0 100644
--- a/regress/roff/shift/bad.in
+++ b/regress/roff/shift/bad.in
@@ -1,5 +1,5 @@
-.\" $OpenBSD: bad.in,v 1.1 2018/08/23 14:16:12 schwarze Exp $
-.TH SHIFT_BAD 1 "August 23, 2018"
+.\" $OpenBSD: bad.in,v 1.2 2022/04/24 13:34:53 schwarze Exp $
+.TH SHIFT_BAD 1 "April 24, 2022"
 .SH NAME
 .B shift-bad
 \(en wrong usage of macro arguments
@@ -22,6 +22,10 @@ argument used after call: "\$1"
 .de mym
 .shift badarg
 after shift badarg: "\\$1"
+.br
+.shift -1
+after shift \-1: "\\$1"
+.br
 .shift 2
 after excessive shift: \\n(.$ "\\$1"
 ..
diff --git a/regress/roff/shift/bad.out_ascii b/regress/roff/shift/bad.out_ascii
index 2d6a2922..ddb8c521 100644
--- a/regress/roff/shift/bad.out_ascii
+++ b/regress/roff/shift/bad.out_ascii
@@ -14,8 +14,10 @@ DDEESSCCRRIIPPTTIIOONN
 
        argument used after call: ""
 
-       after shift badarg: "arg2" after excessive shift: 0 ""
+       after shift badarg: "arg2"
+       after shift -1: "arg2"
+       after excessive shift: 0 ""
 
        final text
 
-OpenBSD                         August 23, 2018                   SHIFT_BAD(1)
+OpenBSD                         April 24, 2022                    SHIFT_BAD(1)
diff --git a/regress/roff/shift/bad.out_lint b/regress/roff/shift/bad.out_lint
index 1f696fc8..b122415e 100644
--- a/regress/roff/shift/bad.out_lint
+++ b/regress/roff/shift/bad.out_lint
@@ -3,5 +3,6 @@ mandoc: bad.in:15:2: ERROR: ignoring request outside macro: shift
 mandoc: bad.in:17:31: ERROR: argument number is not numeric: \$x
 mandoc: bad.in:19:28: ERROR: using macro argument outside macro: \$1
 mandoc: bad.in:20:2: ERROR: ignoring request outside macro: shift
-mandoc: bad.in:28:8: ERROR: argument is not numeric, using 1: shift badarg
-mandoc: bad.in:28:9: ERROR: excessive shift: 2, but max is 1
+mandoc: bad.in:32:8: ERROR: argument is not numeric, using 1: shift badarg
+mandoc: bad.in:32:8: ERROR: negative argument, using 0: shift -1
+mandoc: bad.in:32:8: ERROR: excessive shift: 2, but max is 1
diff --git a/roff.c b/roff.c
index bd222d56..29166fd4 100644
--- a/roff.c
+++ b/roff.c
@@ -1,4 +1,4 @@
-/* $Id: roff.c,v 1.381 2022/04/13 13:19:34 schwarze Exp $ */
+/* $Id: roff.c,v 1.382 2022/04/24 13:38:46 schwarze Exp $ */
 /*
  * Copyright (c) 2010-2015, 2017-2022 Ingo Schwarze <schwarze@openbsd.org>
  * Copyright (c) 2008-2012, 2014 Kristaps Dzonsons <kristaps@bsd.lv>
@@ -3870,8 +3870,9 @@ static int
 roff_shift(ROFF_ARGS)
 {
 	struct mctx	*ctx;
-	int		 levels, i;
+	int		 argpos, levels, i;
 
+	argpos = pos;
 	levels = 1;
 	if (buf->buf[pos] != '\0' &&
 	    roff_evalnum(r, ln, buf->buf, &pos, &levels, 0) == 0) {
@@ -3886,9 +3887,13 @@ roff_shift(ROFF_ARGS)
 	ctx = r->mstack + r->mstackpos;
 	if (levels > ctx->argc) {
 		mandoc_msg(MANDOCERR_SHIFT,
-		    ln, pos, "%d, but max is %d", levels, ctx->argc);
+		    ln, argpos, "%d, but max is %d", levels, ctx->argc);
 		levels = ctx->argc;
 	}
+	if (levels < 0) {
+		mandoc_msg(MANDOCERR_ARG_NEG, ln, argpos, "shift %d", levels);
+		levels = 0;
+	}
 	if (levels == 0)
 		return ROFF_IGN;
 	for (i = 0; i < levels; i++)