Sunday, September 19, 2021

cooledit printf(%n) fixes

Hi Marc, hi Naddy,

Christian Weisgerber wrote on Sun, Sep 19, 2021 at 09:23:18PM +0200:

[...]
> Let's focus on fixing bugs!
> This includes the list of remaining ports with %n warnings:
>
> editors/cooledit
[...]

I think the patch appended below fixes those crashes in cooledit
that are directly related to printf(%n).

OK?

Note that the port is very low quality in general. The code is full
of insecure idioms and the program is full of bugs that make even
minimal testing rather painful. I don't think anybody can use this
port for serious work in its present state.

* It sometimes has outrageous startup times, up to around 20 seconds
on my notebook.
* Putting it into the background with Ctrl-Z does not work.
* Sometimes, it just freezes totally.
* When assigning keys, it sometimes closes the window that is
intended to capture the key combination when pressing a modifier
key, which means that binding keys sometimes does not work.
* Sometimes, it segfaults on exit (plain segfault, not SIGABRT).
* And that is merely what i stumbled across without even trying
to use it for real...

There are many other places in the codebase where %n occurs, but i
failed to find any others related to printf(3). It has large numbers
of its own printf-like wrapper functions and clearly sometimes assembles
format strings dynamically in temporary buffers, but i did not find
indications that %n might be put into such dynamically-built format
strings. Consequenly, i'm not completely sure that this patch resolves
all printf(%n) issues in the code base, but it does not seem unlikely
either. Even if were not 100% complete, committing this would seem
better than doing nothing.

Yours,
Ingo


Index: Makefile
===================================================================
RCS file: /cvs/ports/editors/cooledit/Makefile,v
retrieving revision 1.48
diff -u -p -r1.48 Makefile
--- Makefile 23 Feb 2021 19:39:21 -0000 1.48
+++ Makefile 19 Sep 2021 20:40:00 -0000
@@ -3,7 +3,7 @@
COMMENT = easy to use, graphical editor

DISTNAME = cooledit-3.17.17
-REVISION = 4
+REVISION = 5

CATEGORIES = editors

Index: patches/patch-editor_editoptions_c
===================================================================
RCS file: patches/patch-editor_editoptions_c
diff -N patches/patch-editor_editoptions_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-editor_editoptions_c 19 Sep 2021 20:40:00 -0000
@@ -0,0 +1,28 @@
+$OpenBSD$
+
+Do not use sprintf(3) %n.
+
+Index: editor/editoptions.c
+--- editor/editoptions.c.orig
++++ editor/editoptions.c
+@@ -378,15 +378,15 @@ int save_user_defined_keys (struct key_list k_list[],
+
+ for (i = 0; k_list[i].key_name[0]; i++) {
+ if (k_list[i].keycode2) {
+- sprintf (p, "%s\t%x %x %x %x %x %x\n%n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0,
+- k_list[i].state1, k_list[i].keycode1, k_list[i].state2, k_list[i].keycode2, &n);
++ n = sprintf (p, "%s\t%x %x %x %x %x %x\n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0,
++ k_list[i].state1, k_list[i].keycode1, k_list[i].state2, k_list[i].keycode2);
+ p += n;
+ } else if (k_list[i].keycode1) {
+- sprintf (p, "%s\t%x %x %x %x\n%n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0,
+- k_list[i].state1, k_list[i].keycode1, &n);
++ n = sprintf (p, "%s\t%x %x %x %x\n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0,
++ k_list[i].state1, k_list[i].keycode1);
+ p += n;
+ } else if (k_list[i].keycode0) {
+- sprintf (p, "%s\t%x %x\n%n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0, &n);
++ n = sprintf (p, "%s\t%x %x\n", k_list[i].key_name, k_list[i].state0, k_list[i].keycode0);
+ p += n;
+ }
+ }
Index: patches/patch-editor_options_c
===================================================================
RCS file: patches/patch-editor_options_c
diff -N patches/patch-editor_options_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-editor_options_c 19 Sep 2021 20:40:00 -0000
@@ -0,0 +1,35 @@
+$OpenBSD$
+
+Do not use sprintf(3) %n.
+
+Index: editor/options.c
+--- editor/options.c.orig
++++ editor/options.c
+@@ -405,22 +405,22 @@ int save_setup (const char *file)
+
+ for (i = 0; string_options[i].name; i++) {
+ if (*string_options[i].value) {
+- sprintf (p, "%s = %s\n%n", string_options[i].name, *string_options[i].value, &r);
++ r = sprintf (p, "%s = %s\n", string_options[i].name, *string_options[i].value);
+ p += r;
+ }
+ }
+ for (i = 0; integer_options[i].name; i++) {
+ if (integer_options[i].type == TYPE_HIDDEN_HEX_VALUE)
+- sprintf (p, "%s = 0x%X\n%n", integer_options[i].name, *integer_options[i].value, &r);
++ r = sprintf (p, "%s = 0x%X\n", integer_options[i].name, *integer_options[i].value);
+ else
+- sprintf (p, "%s = %d\n%n", integer_options[i].name, *integer_options[i].value, &r);
++ r = sprintf (p, "%s = %d\n", integer_options[i].name, *integer_options[i].value);
+ p += r;
+ }
+ for (i = 0; color_options[i].name; i++) {
+ if (color_options[i].cname)
+- sprintf (p, "%s = %s\n%n", color_options[i].name, color_options[i].cname, &r);
++ r = sprintf (p, "%s = %s\n", color_options[i].name, color_options[i].cname);
+ else
+- sprintf (p, "%s = %d\n%n", color_options[i].name, *color_options[i].value, &r);
++ r = sprintf (p, "%s = %d\n", color_options[i].name, *color_options[i].value);
+ p += r;
+ }
+ *p = 0;
Index: patches/patch-editor_shell_c
===================================================================
RCS file: patches/patch-editor_shell_c
diff -N patches/patch-editor_shell_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-editor_shell_c 19 Sep 2021 20:40:00 -0000
@@ -0,0 +1,25 @@
+$OpenBSD$
+
+Do not use printf(3) %n.
+
+Index: editor/shell.c
+--- editor/shell.c.orig
++++ editor/shell.c
+@@ -928,7 +928,7 @@ static void save_scripts (void)
+ for (r = t; *r; r++)
+ if (*r == '\n')
+ *r = 129; /* replace newlines with 129 */
+- sprintf (p, "%s\n%s\n%c\n%d\n%d\n%s\n%d\n%s\n%s\n%n",
++ n = sprintf (p, "%s\n%s\n%c\n%d\n%d\n%s\n%d\n%s\n%s\n",
+ scripts[i]->name ? scripts[i]->name : 0,
+ scripts[i]->menu ? scripts[i]->menu : 0,
+ scripts[i]->menu_hot_key ? scripts[i]->menu_hot_key : ' ',
+@@ -937,7 +937,7 @@ static void save_scripts (void)
+ scripts[i]->prompt ? scripts[i]->prompt : "",
+ (int) scripts[i]->options,
+ scripts[i]->last_options ? scripts[i]->last_options : "",
+- t, &n);
++ t);
+ free (t);
+ p += n;
+ i++;

No comments:

Post a Comment