#1092

`utf16_buf` is a two-element array, not sixteen (comma for semicolon in `get_char_for_key`)

Open3x3xX3N0Nopened 3mo ago

The line

src/event/sys/windows/parse.rs:147, in get_char_for_key:

let mut utf16_buf = [0u16, 16];

That is an array literal of two elements — [0u16, 16u16] — where [0u16; 16] (sixteen zeros) was intended. A comma where a semicolon belongs.

Everything around it says sixteen was meant: the name, and the sibling buffer two lines above, which is written correctly with a semicolon:

let key_state = [0u8; 256];        // line 146 — correct
let mut utf16_buf = [0u16, 16];    // line 147 — two elements

This is capacity loss, not a buffer overflow

Worth stating plainly and up front, because "wrong-sized buffer passed to a Win32 API" reads like a memory-safety report and this is not one. The buffer reaches ToUnicodeEx at parse.rs:168-178 as:

ToUnicodeEx(
    virtual_key_code,
    virtual_scan_code,
    key_state.as_ptr(),
    utf16_buf.as_mut_ptr(),
    utf16_buf.len() as i32,   // <-- derived from the ACTUAL array: 2
    dont_change_kernel_keyboard_state,
    active_keyboard_layout,
)

The pointer and the length are derived from the same array, so Win32 is truthfully told cchBuff = 2 and cannot write past the end. That agreement is precisely what saves it. Had the author passed a literal 16 alongside the mis-sized array, this would instead be a straightforward stack buffer overflow.

The stale second element is also harmless: utf16_buf starts as [0, 16], but the decode at parse.rs:186 is bounded by what the API reported writing — decode_utf16(utf16_buf.into_iter().take(ret as usize)) — so index 1 is read only if ToUnicodeEx overwrote it.

What it actually costs

ToUnicodeEx gets two UTF-16 code units of space where sixteen was intended. Any key whose translation needs more than two units cannot be represented. The existing guards then convert that into a silently dropped keystroke rather than anything visible:

  • ret < 1 returns None (parse.rs:182) — covers the dead-key and no-character cases.
  • A translation yielding more than one char returns None (parse.rs:186-191).

So the user-visible symptom is: on Windows, certain keys produce no KeyEvent at all.

Repro shape

This needs a layout whose translation exceeds two UTF-16 units, so it is not reproducible on a plain US layout — worth saying, since "works for me" is the likely first response:

  1. Windows, a crossterm consumer reading event::read() with the events feature.
  2. Select a keyboard layout that produces multi-unit output — a dead-key sequence (e.g. a US-International or Vietnamese layout where an accent key composes with the following vowel), or any layout where ToUnicodeEx returns more than two u16s for one key.
  3. Press the composing sequence. Expected: a KeyEvent carrying the composed character. Actual: no event for that key.
  4. Change line 147 to [0u16; 16] and the same sequence produces the event.

A tighter unit-level repro, if preferred: call get_char_for_key with a KeyEventRecord whose virtual key/scan code map to such a translation under the active layout, and observe None before the fix and Some(ch) after.

Fix

One character:

let mut utf16_buf = [0u16; 16];

Sixteen matches the surrounding intent and is comfortably above what ToUnicodeEx returns for any single key. Nothing else changes — len() continues to supply cchBuff, so the pointer and length stay in agreement.

Provenance

Found by a line-by-line read of the crate for a supply-chain certification, not by hitting the bug in use. Single reader; no adversarial pass was run on this finding, and the reachability statement above is reasoned from the code rather than observed on a live exotic layout — the repro shape is offered as the check, not as something already performed. Recorded that way deliberately: the pointer/length argument is verifiable by reading, the dropped-keystroke consequence is not yet measured.

Upstream context

Read against 0.28.1; checked against the default branch before filing — the line is now src/event/sys/windows/parse.rs:139 and still reads let mut utf16_buf = [0u16, 16];, so this is current rather than a stale read.

Possibly related, offered as a pointer and not as a claim: #1072 reports Windows key input being silently dropped in this same file for a different reason (surrogate-pair handling). The two are independent mechanisms that produce the same user-visible symptom, so a fix for one will not resolve the other.

Activity

0

No activity yet.