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

Unicode EM DASH and "░░▒▓█" symbols corrupted when copying #2845

Closed
starius opened this issue Jun 4, 2017 · 19 comments · Fixed by QubesOS/qubes-gui-agent-linux#117
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: major Priority: major. Between "default" and "critical" in severity. r4.0-bullseye-stable r4.0-buster-stable r4.0-centos7-stable r4.0-centos8-stable r4.0-fc31-stable r4.0-fc32-stable r4.0-fc33-stable r4.0-stretch-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.

Comments

@starius
Copy link

starius commented Jun 4, 2017

Qubes OS version:

R3.2

Affected TemplateVMs:

fedora-23, debian-8


Expected behavior:

I copied and pasted a text that includes "—" symbol (hex "e2 80 94") using Ctrl+Shift+C and Ctrl+Shift+V. I expected my text to be pasted as-is, but the dash symbol corrupted (see below).

Actual behavior:

The symbol was pasted as "�" (hex "c3 a2 c2 80 c2 94").

Steps to reproduce the behavior:

In one VM:

$ echo '0000000: e280 940a' | xxd -r
—

select the symbol with mouse, click right mouse button, choose "Copy". Then press Ctrl+Shift+C.

In another VM: press Ctrl+Shift+V. Open a terminal or a text editor (gedit), click right mouse button, choose "Paste". The symbol pasted corrupted:

dash

General notes:

Info about the symbol:

$ unicode —
U+2014 EM DASH
UTF-8: e2 80 94  UTF-16BE: 2014  Decimal: —
— (—)
Uppercase: U+2014
Category: Pd (Punctuation, Dash)
Bidi: ON (Other Neutrals)

See also http://www.fileformat.info/info/unicode/char/2014/index.htm

@andrewdavidwong andrewdavidwong added T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists. C: core P: minor Priority: minor. The lowest priority, below "default." labels Jun 4, 2017
@andrewdavidwong andrewdavidwong added this to the Release 3.2 updates milestone Jun 4, 2017
@qubesuser
Copy link

qubesuser commented Jun 19, 2017

I'm seeing this issue too and it seems that the data goes correctly to dom0, and the problem lies in pasting.

In particular, as strange as it looks, it seems that the Xutf8TextListToTextProperty function in Xlib is just broken.

Here's a test program:

#include <X11/Xlib.h>
#include <X11/Xutil.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char** argv)
{
	Display* dpy = XOpenDisplay(0);
	char* str = argv[1];
	XTextProperty ct;
        int ret = Xutf8TextListToTextProperty(dpy, (char **)&str, 1, XUTF8StringStyle, &ct);
	printf("%i %li %li %li %i %li\n", ret, strlen(str), strlen((char*)ct.value), ct.encoding, ct.format, ct.nitems);
	return 0;
}
$ ./a.out abcdefghij
0 10 10 246 8 10
$ ./a.out abcdefghij$'\xe2\x80\x94'
0 13 10 246 8 10

So it just silently drops some UTF-8 characters for no reason, despite the fact that it should be doing no conversion at all! This is with the Xlib shipped in Ubuntu 17.04

It seems that GTK does not use Xlib to convert text, and instead does nothing for UTF-8 -> UTF-8, while for STRING it does not convert to Latin-1, but rather treats STRING as Compound Text and thus removes C0 and C1 characters.

The simplest thing for Qubes is probably to only support the UTF8_STRING target and just copy the data verbatim there.

@andrewdavidwong andrewdavidwong added P: major Priority: major. Between "default" and "critical" in severity. and removed P: minor Priority: minor. The lowest priority, below "default." labels Aug 11, 2018
@andrewdavidwong
Copy link
Member

@zaoqi
Copy link

zaoqi commented Jan 21, 2020

░░▒▓█ also corruptes

@jamke
Copy link

jamke commented Oct 12, 2020

░░▒▓█ also corruptes

On R4.0 this is not copied properly either for me. @andrewdavidwong can you please update the issue to reflect this?

@andrewdavidwong andrewdavidwong changed the title Unicode EM DASH symbol corrupted when copying Unicode EM DASH and "░░▒▓█" symbols corrupted when copying Oct 13, 2020
@andrewdavidwong andrewdavidwong added the needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. label Oct 13, 2020
@andrewdavidwong
Copy link
Member

░░▒▓█ also corruptes

On R4.0 this is not copied properly either for me. @andrewdavidwong can you please update the issue to reflect this?

Updated. By the way, I'm not familiar with these symbols, and searching didn't turn up any useful results. What are they?

@tokideveloper
Copy link

@andrewdavidwong

By the way, I'm not familiar with these symbols, and searching didn't turn up any useful results. What are they?

Looking up using gucharmap, these symbols are "Block Elements":

  • U+2591 LIGHT SHADE (25%)
  • U+2592 MEDIUM SHADE (50%)
  • U+2593 DARK SHADE (75%)
  • U+2588 FULL BLOCK (solid)

Useful for e.g. gradients from one color to another.

@jamke
Copy link

jamke commented Oct 13, 2020

Updated. By the way, I'm not familiar with these symbols, and searching didn't turn up any useful results. What are they?

Thank you.

I believe these symbols are used to make GUI in console, e.g. in interface (and copying progress bar in particular) in mc (Midnight Commander).

@andrewdavidwong
Copy link
Member

Ah, interesting. Thanks for the explanation.

@DemiMarie
Copy link

The bug is indeed in Xutf8TextListToTextProperty, and the only workaround I can find is to use XStringListToTextProperty instead. That means that we will need to check for UTF-8 validity ourselves, to avoid invoking undefined behavior and creating a security vulnerability.

Since the clipboard often contains sensitive data such as passwords, a constant-time check is strongly preferred. Anyone an expert in bitsliced algorithms?

@marmarek
Copy link
Member

Since the clipboard often contains sensitive data such as passwords, a constant-time check is strongly preferred.

Is it really observable by potential attacker? I think it can be observed only by:

  • another application running in the same qube - already having access to the clipboard
  • the user (latency of clipboard operation)
    Especially, I think nothing that could observe that time, wouldn't also have access to the clipboard directly anyway, so I don't think constant-time check is important here.

@marmarek
Copy link
Member

As for UTF-8 validation, we already have such code in gui-daemon: https://github.com/QubesOS/qubes-gui-daemon/blob/master/gui-daemon/xside.c#L1994-L2077

@DemiMarie
Copy link

Since the clipboard often contains sensitive data such as passwords, a constant-time check is strongly preferred.

Is it really observable by potential attacker? I think it can be observed only by:

  • another application running in the same qube - already having access to the clipboard
  • the user (latency of clipboard operation)
    Especially, I think nothing that could observe that time, wouldn't also have access to the clipboard directly anyway, so I don't think constant-time check is important here.

The latency isn’t observable, but the cache impact is.

@DemiMarie
Copy link

As for UTF-8 validation, we already have such code in gui-daemon: https://github.com/QubesOS/qubes-gui-daemon/blob/master/gui-daemon/xside.c#L1994-L2077

I will move this into a library routine so the agent can use it as well.

I checked to see if dom0 validates UTF-8 already; it does not. However, I did find a different bug: strings containing NUL characters are truncated. The X STRING target can support arbitrary binary data.

@andrewdavidwong andrewdavidwong added diagnosed Technical diagnosis has been performed (see issue comments). and removed needs diagnosis Requires technical diagnosis from developer. Replace with "diagnosed" or remove if otherwise closed. labels Nov 28, 2020
marmarek pushed a commit to QubesOS/qubes-gui-agent-linux that referenced this issue Dec 5, 2020
libX11 claims to have Xutf8TextListToTextProperty for handling UTF-8
text, but it is just broken and winds up mangling the text instead.
Instead, use XStringListToTextProperty and validate UTF-8 validity
ourselves.

(cherry picked from commit febd112)

Fix indentation and segfault

GCC doesn’t warn about `char **ptr = { NULL };`, even though it is the
same bug as `char *ptr = ""` 😞

(cherry picked from commit b873646)

Fixes QubesOS/qubes-issues#2845
@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-gui-agent_4.0.32-1 has been pushed to the r4.0 testing repository for the Debian template.
To test this update, first enable the testing repository in /etc/apt/sources.list.d/qubes-*.list by uncommenting the line containing buster-testing (or appropriate equivalent for your template version), then use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package gui-agent-linux has been pushed to the r4.0 testing repository for the CentOS centos8 template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component gui-agent-linux (including package pulseaudio-qubes-4.0.32-1.fc32) has been pushed to the r4.0 testing repository for the Fedora template.
To test this update, please install it with the following command:

sudo yum update --enablerepo=qubes-vm-r4.0-current-testing

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package gui-agent-linux has been pushed to the r4.0 stable repository for the CentOS centos8 template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The package qubes-gui-agent_4.0.32-1+deb10u1 has been pushed to the r4.0 stable repository for the Debian template.
To install this update, please use the standard update command:

sudo apt-get update && sudo apt-get dist-upgrade

Changes included in this update

@qubesos-bot
Copy link

Automated announcement from builder-github

The component gui-agent-linux (including package pulseaudio-qubes-4.0.32-1.fc32) has been pushed to the r4.0 stable repository for the Fedora template.
To install this update, please use the standard update command:

sudo yum update

Changes included in this update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: core diagnosed Technical diagnosis has been performed (see issue comments). P: major Priority: major. Between "default" and "critical" in severity. r4.0-bullseye-stable r4.0-buster-stable r4.0-centos7-stable r4.0-centos8-stable r4.0-fc31-stable r4.0-fc32-stable r4.0-fc33-stable r4.0-stretch-stable T: bug Type: bug report. A problem or defect resulting in unintended behavior in something that exists.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants