Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for a crash under FVWM3 #1902

Closed
wants to merge 1 commit into from
Closed

Fix for a crash under FVWM3 #1902

wants to merge 1 commit into from

Conversation

bsdmp
Copy link

@bsdmp bsdmp commented Sep 30, 2023

Core was generated by `rofi'.
Program terminated with signal SIGSEGV, Segmentation fault. 0 strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
125 movq (%rax),%rdx /* get bytes to check */
(gdb) bt
0 strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
1 0x000008dfab6e1558 in __vfprintf (fp=, fmt0=, ap=) at /usr/src/lib/libc/stdio/vfprintf.c:877
2 0x000008dfab6da5a5 in _libc_vasprintf (str=0x7de1bf300610, fmt=0x8dd3e926e47 "Found window manager: |%s|", ap=0x7de1bf300800) at /usr/src/lib/libc/stdio/vasprintf.c:43
3 0x000008df5bfdfac7 in g_vasprintf () from /usr/local/lib/libglib-2.0.so.4201.10
4 0x000008df5bfa412d in g_strdup_vprintf () from /usr/local/lib/libglib-2.0.so.4201.10
5 0x000008df5bf86b3b in g_logv () from /usr/local/lib/libglib-2.0.so.4201.10
6 0x000008df5bf86a55 in g_log () from /usr/local/lib/libglib-2.0.so.4201.10
7 0x000008dd3e9853b2 in x11_helper_discover_window_manager () at ../source/xcb.c:1639
8 0x000008dd3e984d18 in display_setup (main_loop=0x8df6826b320, bindings=0x8df77e08220) at ../source/xcb.c:1696
9 0x000008dd3e954c03 in main (argc=9, argv=0x7de1bf300a58) at ../source/rofi.c:1021
(gdb) f 7
7 0x000008dd3e9853b2 in x11_helper_discover_window_manager () at ../source/xcb.c:1639
1639 g_debug("Found window manager: |%s|", wtitle.strings);
(gdb) p wtitle.strings
$1 = 0x8dfb80a2f50 "FVWM", '\325' <repeats 12 times>, '\337' <repeats 159 times>, <incomplete sequence \337><error: Cannot access memory at address 0x8dfb80a3000>

wtitle.strings is not NULL terminated, handle it with providing string length

Patch by Omar Polo

https://marc.info/?l=openbsd-ports&m=169606431614242&w=2

Core was generated by `rofi'.
Program terminated with signal SIGSEGV, Segmentation fault.
0  strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
125             movq    (%rax),%rdx             /* get bytes to check */
(gdb) bt
0  strlen () at /usr/src/lib/libc/arch/amd64/string/strlen.S:125
1  0x000008dfab6e1558 in __vfprintf (fp=<optimized out>, fmt0=<optimized out>, ap=<optimized out>) at /usr/src/lib/libc/stdio/vfprintf.c:877
2  0x000008dfab6da5a5 in _libc_vasprintf (str=0x7de1bf300610, fmt=0x8dd3e926e47 "Found window manager: |%s|", ap=0x7de1bf300800) at /usr/src/lib/libc/stdio/vasprintf.c:43
3  0x000008df5bfdfac7 in g_vasprintf () from /usr/local/lib/libglib-2.0.so.4201.10
4  0x000008df5bfa412d in g_strdup_vprintf () from /usr/local/lib/libglib-2.0.so.4201.10
5  0x000008df5bf86b3b in g_logv () from /usr/local/lib/libglib-2.0.so.4201.10
6  0x000008df5bf86a55 in g_log () from /usr/local/lib/libglib-2.0.so.4201.10
7  0x000008dd3e9853b2 in x11_helper_discover_window_manager () at ../source/xcb.c:1639
8  0x000008dd3e984d18 in display_setup (main_loop=0x8df6826b320, bindings=0x8df77e08220) at ../source/xcb.c:1696
9  0x000008dd3e954c03 in main (argc=9, argv=0x7de1bf300a58) at ../source/rofi.c:1021
(gdb) f 7
7  0x000008dd3e9853b2 in x11_helper_discover_window_manager () at ../source/xcb.c:1639
1639            g_debug("Found window manager: |%s|", wtitle.strings);
(gdb) p wtitle.strings
$1 = 0x8dfb80a2f50 "FVWM", '\325' <repeats 12 times>, '\337' <repeats 159 times>, <incomplete sequence \337><error: Cannot access memory at address 0x8dfb80a3000>

wtitle.strings is not NULL terminated, handle it with providing string length

Patch by Omar Polo

https://marc.info/?l=openbsd-ports&m=169606431614242&w=2
@DaveDavenport
Copy link
Collaborator

Thanks, I think this might need a bit more of a fix to be sure as the non null terminated string is used later.. Ill look at it later.

@DaveDavenport
Copy link
Collaborator

Can you test:

diff --git a/source/xcb.c b/source/xcb.c
index 375cfcfc..4e103979 100644
--- a/source/xcb.c
+++ b/source/xcb.c
@@ -1617,7 +1617,7 @@ char *x11_helper_get_window_manager(void) {
         xcb_ewmh_get_wm_name_unchecked(&(xcb->ewmh), wm_win);
     if (xcb_ewmh_get_wm_name_reply(&(xcb->ewmh), cookie, &wtitle, (void *)0)) {
       if (wtitle.strings_len > 0) {
-        retv = g_strdup(wtitle.strings);
+        retv = g_strndup(wtitle.strings, wtitle.strings_len);
       }
       xcb_ewmh_get_utf8_strings_reply_wipe(&wtitle);
     }
@@ -1636,13 +1636,16 @@ static void x11_helper_discover_window_manager(void) {
         xcb_ewmh_get_wm_name_unchecked(&(xcb->ewmh), wm_win);
     if (xcb_ewmh_get_wm_name_reply(&(xcb->ewmh), cookie, &wtitle, (void *)0)) {
       if (wtitle.strings_len > 0) {
-        g_debug("Found window manager: |%s|", wtitle.strings);
-        if (g_strcmp0(wtitle.strings, "i3") == 0) {
+        // Copy the string and add terminating '\0'.
+        char *str = g_strndup(wtitle.strings, wtitle.strings_len);
+        g_debug("Found window manager: |%s|", str);
+        if (g_strcmp0(str, "i3") == 0) {
           current_window_manager =
               WM_DO_NOT_CHANGE_CURRENT_DESKTOP | WM_PANGO_WORKSPACE_NAMES;
-        } else if (g_strcmp0(wtitle.strings, "bspwm") == 0) {
+        } else if (g_strcmp0(str, "bspwm") == 0) {
           current_window_manager = WM_ROOT_WINDOW_OFFSET;
         }
+        g_free(str);
       }
       xcb_ewmh_get_utf8_strings_reply_wipe(&wtitle);
     }

@bsdmp
Copy link
Author

bsdmp commented Sep 30, 2023

100 iterations - no crash

@DaveDavenport
Copy link
Collaborator

Thanks for testing.

@DaveDavenport
Copy link
Collaborator

Added the fix to git.

@DaveDavenport
Copy link
Collaborator

Thanks

Copy link

github-actions bot commented Nov 1, 2023

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants