[GUACAMOLE-149] Potential busy loop in guac_terminal_display_set_columns()

• Type: Bug
• Status: Resolved
• Priority: Critical
• Resolution: Fixed
• Affects Version/s: 0.9.9, 0.9.10-incubating
• Fix Version/s: 0.9.12-incubating
• Component/s: guacamole-server, SSH, Telnet
• Assignee: Mike Jumper
• Reporter: Aiden Luo
• Created: 29/Dec/16 02:50
• Updated: 22/Sep/17 20:27
• Resolved: 03/Feb/17 03:17

Description

The Guacamole terminal emulator can get caught in an endless busy loop if a character in the buffer ends up being set with zero width. The loop within guac_terminal_display_set_columns() advances through columns by the width of the current character, and will never advance if that width is somehow unset.

(gdb) bt
#0  guac_terminal_display_set_columns (display=0x30f5480, row=11, start_column=22, end_column=22, character=0x2cf3b18)
    at display.c:458
#1  0x00007f8b3fa68374 in guac_terminal_commit_cursor (term=term@entry=0x2c0b5b0) at terminal.c:649
#2  0x00007f8b3fa69329 in guac_terminal_flush (terminal=terminal@entry=0x2c0b5b0) at terminal.c:1339
#3  0x00007f8b3fa69440 in guac_terminal_render_frame (terminal=0x2c0b5b0) at terminal.c:489
#4  0x000000000040312d in __guacd_client_output_thread (data=0x2bea930) at client.c:73
#5  0x00007f8b43469184 in start_thread (arg=0x7f8b3c326700) at pthread_create.c:312
#6  0x00007f8b4319637d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:111
(gdb) f 0
#0  guac_terminal_display_set_columns (display=0x30f5480, row=11, start_column=22, end_column=22, character=0x2cf3b18)
    at display.c:458
458             current->type      = GUAC_CHAR_SET;
(gdb) list
453
454         /* For each column in range */
455         for (i = start_column; i <= end_column; i += character->width) {
456
457             /* Set operation */
458             current->type      = GUAC_CHAR_SET;
459             current->character = *character;
460
461             /* Next character */
462             current += character->width;
(gdb) p character->width
$113 = 0
(gdb)

From __guac_terminal_set(), it's clear that having zero width is a legitimate value:

    /* Calculate width in columns */
    width = wcwidth(codepoint);
    if (width < 0)
        width = 1;

    /* Do nothing if glyph is empty */
    if (width == 0)
        return 0;

Issue Links

• is duplicated by: GUACAMOLE-387 Process in dead loop and cpu running 100%