+Cc: MAINTAINER
Hi Theo,
Theo Buehler wrote on Wed, Sep 15, 2021 at 11:59:36PM +0200:
> Straightforward.
OK schwarze@.
Given that we are approaching a lock, i don't think you should wait
for feedback from the MAINTAINER.
Alessandro, i suspect buffer overflow issues in util/misc.c,
function CreateGeometryString(). The arguments are (int) and can
likely be provided from outside the program, but the buffer being
printed to is only 24 bytes long. I didn't poke around long enough
to figure out whether this can crash the program (or worse), but as
the MAINTAINER, it might make sense that you have a closer look.
But that seems unrelated to the present patch.
There is much opportunity for additional cleanup to be done upstream:
for example, strcpy(3) in conjunction with pointer arithmetics is all
over the place, and invariants are quite non-obvious if any exist.
Yours,
Ingo
Testing done:
Before:
$ /obin/ncl tmp.txt
Abort trap (core dumped)
$ /obin/nedit -g 80x24+100+100
Abort trap (core dumped)
After:
$ ncl tmp.txt # works
$ nedit -g 80x24+100+100 # works
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/editors/nedit/Makefile,v
> retrieving revision 1.82
> diff -u -p -r1.82 Makefile
> --- Makefile 12 Apr 2020 14:46:04 -0000 1.82
> +++ Makefile 13 Sep 2021 16:06:53 -0000
> @@ -5,7 +5,7 @@ COMMENT= a fast, compact Motif/X11 plai
> DISTNAME= nedit-5.7
> P_V= 0.5
> EPOCH= 0
> -REVISION = 0
> +REVISION = 1
> DISTFILES= ${DISTNAME}-src${EXTRACT_SUFX} \
> nedit_patterns-${P_V}.tgz:0
>
> Index: patches/patch-source_nc_c
> ===================================================================
> RCS file: /cvs/ports/editors/nedit/patches/patch-source_nc_c,v
> retrieving revision 1.2
> diff -u -p -r1.2 patch-source_nc_c
> --- patches/patch-source_nc_c 28 Feb 2019 23:00:47 -0000 1.2
> +++ patches/patch-source_nc_c 13 Sep 2021 16:04:55 -0000
> @@ -27,3 +27,28 @@ Index: source/nc.c
> #endif /*VMS*/
>
> /* Structure to hold X Resource values */
> +@@ -778,10 +778,10 @@ static void parseCommandLine(int argc, char **argv, Co
> + The "long" cast on strlen() is necessary because size_t
> + is 64 bit on Alphas, and 32-bit on most others. There is
> + no printf format specifier for "size_t", thanx, ANSI. */
> +- sprintf(outPtr, "%d %d %d %d %d %ld %ld %ld %ld\n%n",
> lineNum,
> ++ charsWritten = sprintf(outPtr, "%d %d %d %d %d %ld %ld %ld
> %ld\n", lineNum,
> + read, create, iconic, isTabbed, (long) strlen(path),
> + (long) strlen(toDoCommand), (long) strlen(langMode),
> +- (long) strlen(geometry), &charsWritten);
> ++ (long) strlen(geometry));
> + outPtr += charsWritten;
> + strcpy(outPtr, path);
> + outPtr += strlen(path);
> +@@ -816,9 +816,9 @@ static void parseCommandLine(int argc, char **argv, Co
> + * iconic state (and optional language mode and geometry).
> + */
> + if (toDoCommand[0] != '\0' || fileCount == 0) {
> +- sprintf(outPtr, "0 0 0 %d %d 0 %ld %ld %ld\n\n%n", iconic, tabbed,
> ++ charsWritten = sprintf(outPtr, "0 0 0 %d %d 0 %ld %ld %ld\n\n", iconic,
> tabbed,
> + (long) strlen(toDoCommand),
> +- (long) strlen(langMode), (long) strlen(geometry),
> &charsWritten);
> ++ (long) strlen(langMode), (long) strlen(geometry));
> + outPtr += charsWritten;
> + strcpy(outPtr, toDoCommand);
> + outPtr += strlen(toDoCommand);
> Index: patches/patch-util_misc_c
> ===================================================================
> RCS file: patches/patch-util_misc_c
> diff -N patches/patch-util_misc_c
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-util_misc_c 13 Sep 2021 16:01:48 -0000
> @@ -0,0 +1,37 @@
> +$OpenBSD$
> +
> +Index: util/misc.c
> +--- util/misc.c.orig
> ++++ util/misc.c
> +@@ -1488,25 +1488,25 @@ void CreateGeometryString(char *string, int x, int y,
> + int nChars;
> +
> + if (bitmask & WidthValue) {
> +- sprintf(ptr, "%d%n", width, &nChars);
> ++ nChars = sprintf(ptr, "%d", width);
> + ptr += nChars;
> + }
> + if (bitmask & HeightValue) {
> +- sprintf(ptr, "x%d%n", height, &nChars);
> ++ nChars = sprintf(ptr, "x%d", height);
> + ptr += nChars;
> + }
> + if (bitmask & XValue) {
> + if (bitmask & XNegative)
> +- sprintf(ptr, "-%d%n", -x, &nChars);
> ++ nChars = sprintf(ptr, "-%d", -x);
> + else
> +- sprintf(ptr, "+%d%n", x, &nChars);
> ++ nChars = sprintf(ptr, "+%d", x);
> + ptr += nChars;
> + }
> + if (bitmask & YValue) {
> + if (bitmask & YNegative)
> +- sprintf(ptr, "-%d%n", -y, &nChars);
> ++ nChars = sprintf(ptr, "-%d", -y);
> + else
> +- sprintf(ptr, "+%d%n", y, &nChars);
> ++ nChars = sprintf(ptr, "+%d", y);
> + ptr += nChars;
> + }
> + *ptr = '\0';
