From 5f5319dc2f3b58303456fb69d224d1bd7433ff2f Mon Sep 17 00:00:00 2001 From: Jess Sullivan Date: Fri, 3 Apr 2026 08:10:54 -0400 Subject: [PATCH] fix(security): harden hidraw enumeration, FFI bounds, credential parsing - hid_linux: scan /sys/class/hidraw/ instead of hardcoded 0-15 range, add bounds check in HID descriptor parser, distinguish "no devices" from "devices not accessible" (permissions error) - ffi: add size checks before every memcpy to caller buffers, preventing buffer overrun from malformed device responses - ctap2: bound credential ID length to 1024 bytes (reject implausible) - ctap2.h: add CTAP2_ERR_NOT_ACCESSIBLE (-11) error code --- include/ctap2.h | 1 + src/ctap2.zig | 2 ++ src/ffi.zig | 88 +++++++++++++++++++++++++++++++++++++++-------- src/hid_linux.zig | 71 +++++++++++++++++++++++++++++++++----- 4 files changed, 140 insertions(+), 22 deletions(-) diff --git a/include/ctap2.h b/include/ctap2.h index a878de4..a1f2daa 100644 --- a/include/ctap2.h +++ b/include/ctap2.h @@ -27,6 +27,7 @@ extern "C" { #define CTAP2_ERR_CBOR -8 #define CTAP2_ERR_DEVICE -9 #define CTAP2_ERR_PIN -10 +#define CTAP2_ERR_NOT_ACCESSIBLE -11 // FIDO devices found but not openable (permissions) // Get the number of connected FIDO2 devices. int ctap2_device_count(void); diff --git a/src/ctap2.zig b/src/ctap2.zig index 0fde253..2611f9d 100644 --- a/src/ctap2.zig +++ b/src/ctap2.zig @@ -265,6 +265,8 @@ pub fn parseMakeCredentialResponse(response_data: []const u8) cbor.Error!MakeCre if (auth_data.len < 55) return cbor.Error.Truncated; const cred_id_len = @as(usize, auth_data[53]) << 8 | @as(usize, auth_data[54]); + // CTAP2 credential IDs are typically 16-256 bytes; reject implausible sizes + if (cred_id_len > 1024) return cbor.Error.InvalidCbor; if (auth_data.len < 55 + cred_id_len) return cbor.Error.Truncated; const credential_id = auth_data[55..][0..cred_id_len]; diff --git a/src/ffi.zig b/src/ffi.zig index 77c4034..449b48b 100644 --- a/src/ffi.zig +++ b/src/ffi.zig @@ -23,6 +23,15 @@ const CTAP2_ERR_READ_FAILED: c_int = -7; const CTAP2_ERR_CBOR: c_int = -8; const CTAP2_ERR_DEVICE: c_int = -9; const CTAP2_ERR_PIN: c_int = -10; +const CTAP2_ERR_NOT_ACCESSIBLE: c_int = -11; + +// Maximum output buffer sizes matching the documented C API contract. +// Parsed response functions assume callers allocate at least these sizes. +const MAX_CREDENTIAL_ID: usize = 1024; +const MAX_ATTESTATION_OBJECT: usize = 4096; +const MAX_AUTH_DATA: usize = 4096; +const MAX_SIGNATURE: usize = 1024; +const MAX_USER_HANDLE: usize = 1024; /// Keepalive callback type: receives status byte (1=processing, 2=user presence needed). pub const KeepaliveCallback = ?*const fn (u8) callconv(.c) void; @@ -161,7 +170,10 @@ export fn ctap2_make_credential( const allocator = gpa.allocator(); // Open first FIDO2 device - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Encode CTAP2 makeCredential command @@ -209,7 +221,10 @@ export fn ctap2_get_assertion( const allocator = gpa.allocator(); // Open first FIDO2 device - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Build allow list slices @@ -253,7 +268,10 @@ export fn ctap2_get_info( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); var cmd_buf: [8]u8 = undefined; @@ -300,7 +318,10 @@ export fn ctap2_make_credential_parsed( const allocator = gpa.allocator(); // Open first FIDO2 device - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Encode CTAP2 makeCredential command @@ -337,13 +358,15 @@ export fn ctap2_make_credential_parsed( return @intCast(result.status); } - // Copy credential ID to output buffer + // Copy credential ID to output buffer (bounds-checked) const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; - // Copy attestation object to output buffer + // Copy attestation object to output buffer (bounds-checked) const att_obj = result.attestation_object; + if (att_obj.len > MAX_ATTESTATION_OBJECT) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_attestation_object[0..att_obj.len], att_obj); out_attestation_object_len.* = att_obj.len; @@ -377,7 +400,10 @@ export fn ctap2_get_assertion_parsed( const allocator = gpa.allocator(); // Open first FIDO2 device - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Build allow list slices @@ -426,20 +452,24 @@ export fn ctap2_get_assertion_parsed( return @intCast(result.status); } - // Copy outputs + // Copy outputs (bounds-checked against documented buffer sizes) const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; const auth_data = result.auth_data; + if (auth_data.len > MAX_AUTH_DATA) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_auth_data[0..auth_data.len], auth_data); out_auth_data_len.* = auth_data.len; const sig = result.signature; + if (sig.len > MAX_SIGNATURE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_signature[0..sig.len], sig); out_signature_len.* = sig.len; const user_handle = result.user_handle; + if (user_handle.len > MAX_USER_HANDLE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_user_handle[0..user_handle.len], user_handle); out_user_handle_len.* = user_handle.len; @@ -467,10 +497,12 @@ export fn ctap2_parse_make_credential_response( } const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; const att_obj = result.attestation_object; + if (att_obj.len > MAX_ATTESTATION_OBJECT) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_attestation_object[0..att_obj.len], att_obj); out_attestation_object_len.* = att_obj.len; @@ -512,18 +544,22 @@ export fn ctap2_parse_get_assertion_response( } const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; const auth_data = result.auth_data; + if (auth_data.len > MAX_AUTH_DATA) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_auth_data[0..auth_data.len], auth_data); out_auth_data_len.* = auth_data.len; const sig = result.signature; + if (sig.len > MAX_SIGNATURE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_signature[0..sig.len], sig); out_signature_len.* = sig.len; const user_handle = result.user_handle; + if (user_handle.len > MAX_USER_HANDLE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_user_handle[0..user_handle.len], user_handle); out_user_handle_len.* = user_handle.len; @@ -557,7 +593,10 @@ export fn ctap2_get_pin_retries( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Encode getPINRetries command @@ -607,7 +646,10 @@ export fn ctap2_get_pin_token( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Step 1: getKeyAgreement — get authenticator's ECDH public key @@ -703,7 +745,10 @@ export fn ctap2_make_credential_with_pin( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); var cmd_buf: [2048]u8 = undefined; @@ -752,10 +797,12 @@ export fn ctap2_make_credential_with_pin( } const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; const att_obj = result.attestation_object; + if (att_obj.len > MAX_ATTESTATION_OBJECT) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_attestation_object[0..att_obj.len], att_obj); out_attestation_object_len.* = att_obj.len; @@ -790,7 +837,10 @@ export fn ctap2_get_assertion_with_pin( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Build allow list slices @@ -846,18 +896,22 @@ export fn ctap2_get_assertion_with_pin( } const cred_id = result.credential_id; + if (cred_id.len > MAX_CREDENTIAL_ID) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_credential_id[0..cred_id.len], cred_id); out_credential_id_len.* = cred_id.len; const auth_data = result.auth_data; + if (auth_data.len > MAX_AUTH_DATA) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_auth_data[0..auth_data.len], auth_data); out_auth_data_len.* = auth_data.len; const sig = result.signature; + if (sig.len > MAX_SIGNATURE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_signature[0..sig.len], sig); out_signature_len.* = sig.len; const user_handle = result.user_handle; + if (user_handle.len > MAX_USER_HANDLE) return CTAP2_ERR_BUFFER_TOO_SMALL; @memcpy(out_user_handle[0..user_handle.len], user_handle); out_user_handle_len.* = user_handle.len; @@ -888,7 +942,10 @@ export fn ctap2_make_credential_with_keepalive( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); var cmd_buf: [2048]u8 = undefined; @@ -932,7 +989,10 @@ export fn ctap2_get_assertion_with_keepalive( defer _ = gpa.deinit(); const allocator = gpa.allocator(); - var dev = hid.openFirst(allocator) catch return CTAP2_ERR_NO_DEVICE; + var dev = hid.openFirst(allocator) catch |err| return switch (err) { + error.DevicesNotAccessible => CTAP2_ERR_NOT_ACCESSIBLE, + else => CTAP2_ERR_NO_DEVICE, + }; defer dev.close(); // Build allow list slices (same pattern as ctap2_get_assertion) diff --git a/src/hid_linux.zig b/src/hid_linux.zig index 86db9f4..c8b57e5 100644 --- a/src/hid_linux.zig +++ b/src/hid_linux.zig @@ -9,6 +9,7 @@ const posix = std.posix; pub const Error = error{ NoDeviceFound, + DevicesNotAccessible, OpenFailed, WriteFailed, ReadFailed, @@ -87,22 +88,29 @@ fn isFidoDevice(path: []const u8) bool { const item_type = (item >> 2) & 0x03; const item_tag = (item >> 4) & 0x0F; + // Per HID spec: size field 3 means 4 data bytes + const data_bytes: usize = if (item_size == 3) 4 else item_size; + + // Bounds check: ensure data bytes are within descriptor + if (i + 1 + data_bytes > desc.len) break; + if (item_type == 1 and item_tag == 0 and item_size == 2) { // Usage Page (2 bytes) - if (i + 2 < desc.len) { - const page = @as(u16, desc[i + 1]) | (@as(u16, desc[i + 2]) << 8); - if (page == FIDO_USAGE_PAGE) return true; - } + const page = @as(u16, desc[i + 1]) | (@as(u16, desc[i + 2]) << 8); + if (page == FIDO_USAGE_PAGE) return true; } // Advance past this item - i += 1 + @as(usize, if (item_size == 3) 4 else item_size); + i += 1 + data_bytes; } return false; } /// Enumerate connected FIDO2 USB HID devices. +/// Scans /sys/class/hidraw/ to discover all hidraw devices (not limited +/// to a fixed range). Returns DevicesNotAccessible if FIDO devices are +/// found but none could be opened (likely a permissions issue). pub fn enumerate(allocator: std.mem.Allocator) ![]Device { var devices: std.ArrayList(Device) = .empty; errdefer { @@ -110,9 +118,56 @@ pub fn enumerate(allocator: std.mem.Allocator) ![]Device { devices.deinit(allocator); } - // Scan /dev/hidraw0 through /dev/hidraw15 - var idx: u8 = 0; - while (idx < 16) : (idx += 1) { + // Scan /sys/class/hidraw/ for actual device entries + var fido_found: usize = 0; + var open_failed: usize = 0; + const sysfs_dir = std.fs.openDirAbsolute("/sys/class/hidraw", .{ .iterate = true }) catch { + // Fallback: scan /dev/hidraw0..255 if sysfs is unavailable + return enumerateFallback(allocator); + }; + // sysfs_dir is const, close via a mutable copy after iteration + var dir = sysfs_dir; + defer dir.close(); + var iter = dir.iterate(); + + while (iter.next() catch null) |entry| { + if (!std.mem.startsWith(u8, entry.name, "hidraw")) continue; + + var path_buf: [64]u8 = undefined; + const path = std.fmt.bufPrint(&path_buf, "/dev/{s}", .{entry.name}) catch continue; + + if (!isFidoDevice(path)) continue; + fido_found += 1; + + const fd = posix.open(path, .{ .ACCMODE = .RDWR }, 0) catch { + open_failed += 1; + continue; + }; + + var dev = Device{ .fd = fd }; + @memcpy(dev.path[0..path.len], path); + try devices.append(allocator, dev); + } + + // Distinguish "no FIDO devices" from "found but can't open" (permissions) + if (devices.items.len == 0 and fido_found > 0 and open_failed == fido_found) { + return Error.DevicesNotAccessible; + } + + return try devices.toOwnedSlice(allocator); +} + +/// Fallback enumeration when /sys/class/hidraw is not available. +/// Scans /dev/hidraw0 through /dev/hidraw255. +fn enumerateFallback(allocator: std.mem.Allocator) ![]Device { + var devices: std.ArrayList(Device) = .empty; + errdefer { + for (devices.items) |*dev| dev.close(); + devices.deinit(allocator); + } + + var idx: u16 = 0; + while (idx < 256) : (idx += 1) { var path_buf: [32]u8 = undefined; const path = std.fmt.bufPrint(&path_buf, "/dev/hidraw{d}", .{idx}) catch continue;