linebender / druid

A data-first Rust-native UI design toolkit.
https://linebender.org/druid/
Apache License 2.0
9.52k stars 568 forks source link

Window name is not null terminated in X11 #2372

Closed IronOxidizer closed 1 year ago

IronOxidizer commented 1 year ago

This is set here: https://github.com/linebender/druid/blob/5fa4ce51ed3d74640388de6385f135c50d346c8d/druid/src/app.rs#L107

This is reproducible with the following C code snippet:

xcb_get_property_reply_t *name_reply = xcb_get_property_reply(conn, xcb_get_property(conn, 0, window, XCB_ATOM_WM_NAME, XCB_ATOM_STRING, 0, 256), NULL);
if (name_reply) {
    char *name = xcb_get_property_value(name_reply);
    ...

name for a druid application is 61 70 70 2d 6e 61 6d 65 11 01 which is app-name

It should be 61 70 70 2d 6e 61 6d 65 00 which is app-name

This is only an issue for Druid applications. I have not been able to reproduce it in a hello-world application for Iced, GTK, or QT.

jneem commented 1 year ago

Ok this is interesting, and I haven't found an authoritative source for what the correct behavior is. In the X11 protocol itself, the string is encoded with a length so it doesn't need to be null-terminated. And I found this thread where dwm is discussing whether WM_NAME should be null-terminated. I think they end up deciding not to, but it isn't clear whether this applies to _NET_WM_NAME. We should check to see exactly what GTK and Qt do here...

psychon commented 1 year ago

I propose to close this bug report as invalid.

This is reproducible with the following C code snippet:

That C code is wrong. libxcb only exposes the X11 wire protocol and X11 does not null-terminate its strings. Please fix your code to use xcb_get_property_value_length() and only read that many bytes from the result of xcb_get_property_value(). Copy the data if you need to add a null terminator.

Random code sample: https://github.com/awesomeWM/awesome/blob/0e5fc4575ab0adbae75908cb49937d9cf63437ec/common/xutil.h#L46-L51

However, the X11 protocol pads many things to a multiple of four bytes (in this case: Replies are always a multiple of four bytes long) and the padding is AFAIK always a null byte, so these kind of bugs are relatively common in code using libxcb. It goes wrong too seldomly.

Here is a screenshot of Wireshark for a GetPropertyReply for a window with title 1234 (dunno why Wireshark didn't recognise the reply type). You see that the "1234" is right at the end of the packet without a \0 afterwards:

1234

And here is the same thing for a window with title "1234567". This is one byte short for a multiple of four and thus padded with a zero byte.

1234567

We should check to see exactly what GTK and Qt do here...

Well... random example:

$ xtrace gvim | grep ChangeProperty | grep WM_NAME
No display name to create specified, trying :9
Got connection from unknown(local)
Got connection from unknown(local)

(gvim:11916): dbind-WARNING **: 14:24:24.357: AT-SPI: Error retrieving accessibility bus address: org.freedesktop.DBus.Error.ServiceUnknown: The name org.a11y.Bus was not provided by any .service files
000:<:0071: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000001 property=0x163("_NET_WM_NAME") type=0x154("UTF8_STRING") data=0x67,0x76,0x69,0x6d;
000:<:0072: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000001 property=0x27("WM_NAME") type=0x1f("STRING") data='gvim'
000:<:00a1: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000003 property=0x163("_NET_WM_NAME") type=0x154("UTF8_STRING") data=0x56,0x69,0x6d;
000:<:00a2: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000003 property=0x27("WM_NAME") type=0x1f("STRING") data='Vim'
000:<:00ab: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000001 property=0x163("_NET_WM_NAME") type=0x154("UTF8_STRING") data=0x56,0x69,0x6d;
000:<:00ac: 28: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000001 property=0x27("WM_NAME") type=0x1f("STRING") data='Vim'
000:<:01b1: 44: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000003 property=0x163("_NET_WM_NAME") type=0x154("UTF8_STRING") data=0x5b,0x55,0x6e,0x62,0x65,0x6e,0x61,0x6e,0x6e,0x74,0x5d,0x20,0x20,0x2d,0x20,0x56,0x49,0x4d;
000:<:01b2: 44: Request(18): ChangeProperty mode=Replace(0x00) window=0x04000003 property=0x27("WM_NAME") type=0x1f("STRING") data='[Unbenannt]  - VIM'

Clearly no zero bytes in the data of _NET_WM_NAME. Sadly, xtrace decodes the WM_NAME request and does not show the raw contents. But I tell you: There is no zero byte in there either.

(I don't think I have any Qt apps laying around to test this with, but I can guarantee you that they also do not add that byte to the end of their window titles.)

jneem commented 1 year ago

Thanks for the detailed information!