diff --git a/src/terminal/kitty/clipboard.zig b/src/terminal/kitty/clipboard.zig index 55c42fe8e..b848b1c04 100644 --- a/src/terminal/kitty/clipboard.zig +++ b/src/terminal/kitty/clipboard.zig @@ -11,6 +11,9 @@ //! metadata section), an unknown or missing `type`, and invalid //! base64 in `mime`, `name`, or `pw` all silently drop the request //! with no response. +//! * Decoded metadata and MIME-list payloads must be valid UTF-8. An +//! invalid value on `wdata` or `walias`, or a `walias` without a +//! target MIME type, aborts an in-flight write with EINVAL. //! * `mime`, `name`, and `pw` metadata values are base64-encoded UTF-8; //! everything else is verbatim. Unknown keys are ignored. //! * `id` is sanitized by stripping characters outside [a-zA-Z0-9-_+.] diff --git a/src/terminal/kitty/clipboard_command.zig b/src/terminal/kitty/clipboard_command.zig index 0bf5a1aa7..f5a6b1753 100644 --- a/src/terminal/kitty/clipboard_command.zig +++ b/src/terminal/kitty/clipboard_command.zig @@ -70,78 +70,108 @@ pub const Metadata = struct { /// Parse the metadata field. The raw value is expected to be exactly /// the metadata (prefix and payload and separators stripped out). /// - /// A null result means it was invalid but without any response. - /// Silently drop the OSC. + /// A null result means the packet should be silently dropped. An + /// InvalidValue error means a decoded textual value was not valid + /// UTF-8 or exceeded a local safety limit. Callers use the raw + /// operation to decide whether that invalid value aborts an in-flight + /// write transaction. pub fn parse( alloc: Allocator, raw: []const u8, - ) Allocator.Error!?Metadata { - var op_raw: ?[]const u8 = null; - var result: Metadata = .{ .op = undefined }; + ) error{ OutOfMemory, InvalidValue }!?Metadata { + const fields = Raw.parse(raw) orelse return null; + var result: Metadata = .{ + .op = Operation.init(fields.op orelse return null) orelse return null, + .loc = if (std.mem.eql(u8, fields.loc, "primary")) + .primary + else + .standard, + .id = try sanitizeId(alloc, fields.id), + }; - // Note this loop visits every record even though an empty raw - // string yields a single empty record: that record has no '=' - // and correctly drops the packet, matching kitty which requires - // at least a valid `type` record. - var it = std.mem.splitScalar(u8, raw, ':'); - while (it.next()) |record| { - // Every record must be key=value. Any single invalid record - // is dropped, matching Kitty's behavior. - const eql_idx = std.mem.indexOfScalar(u8, record, '=') orelse return null; - const key = record[0..eql_idx]; - const value = record[eql_idx + 1 ..]; - - if (std.mem.eql(u8, key, "type")) { - // Validated after the loop: a duplicate key's last - // occurrence wins. This isn't specified but its how Kitty - // works. - op_raw = value; - } else if (std.mem.eql(u8, key, "loc")) { - result.loc = if (std.mem.eql(u8, value, "primary")) - .primary - else - .standard; - } else if (std.mem.eql(u8, key, "id")) { - result.id = try sanitizeId(alloc, value); - } else if (std.mem.eql(u8, key, "mime")) { - result.mime = decodeValue( - alloc, - value, - max_mime_len, - ) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - error.Overflow, error.Invalid => return null, - }; - } else if (std.mem.eql(u8, key, "pw")) { - result.pw = decodeValue( - alloc, - value, - max_pw_len, - ) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - // An over-long password behaves as if none was - // given: it can never match a stored grant. - error.Overflow => "", - error.Invalid => return null, - }; - } else if (std.mem.eql(u8, key, "name")) { - result.name = decodeValue( - alloc, - value, - max_name_len, - ) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - error.Overflow, error.Invalid => return null, - }; - } - // Unknown keys are ignored. - } - - // A missing or unknown operation drops the request. - result.op = Operation.init(op_raw orelse return null) orelse return null; + result.mime = decodeValue( + alloc, + fields.mime, + max_mime_len, + ) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + // Base64 acceptance is intentionally left unchanged while the + // protocol's exact requirements are being specified. + error.InvalidBase64 => return null, + error.Overflow, error.InvalidUtf8 => return error.InvalidValue, + }; + result.pw = decodeValue( + alloc, + fields.pw, + max_pw_len, + ) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.InvalidBase64 => return null, + // An over-long password behaves as if none was given: it can + // never match a stored grant. + error.Overflow => "", + error.InvalidUtf8 => return error.InvalidValue, + }; + result.name = decodeValue( + alloc, + fields.name, + max_name_len, + ) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.InvalidBase64 => return null, + error.Overflow, error.InvalidUtf8 => return error.InvalidValue, + }; return result; } + /// Return the last recognized operation from syntactically valid raw + /// metadata. This intentionally does not decode any values, so callers + /// can still classify an InvalidValue parse error. + pub fn operation(raw: []const u8) ?Operation { + const fields = Raw.parse(raw) orelse return null; + return Operation.init(fields.op orelse return null); + } + + const Raw = struct { + op: ?[]const u8 = null, + loc: []const u8 = "", + id: []const u8 = "", + mime: []const u8 = "", + pw: []const u8 = "", + name: []const u8 = "", + + fn parse(raw: []const u8) ?Raw { + var result: Raw = .{}; + + // This visits every record even though an empty raw string + // yields one empty record. Validating the complete structure + // before decoding also ensures the last duplicate value wins. + var it = std.mem.splitScalar(u8, raw, ':'); + while (it.next()) |record| { + const eql_idx = std.mem.indexOfScalar(u8, record, '=') orelse return null; + const key = record[0..eql_idx]; + const value = record[eql_idx + 1 ..]; + + if (std.mem.eql(u8, key, "type")) { + result.op = value; + } else if (std.mem.eql(u8, key, "loc")) { + result.loc = value; + } else if (std.mem.eql(u8, key, "id")) { + result.id = value; + } else if (std.mem.eql(u8, key, "mime")) { + result.mime = value; + } else if (std.mem.eql(u8, key, "pw")) { + result.pw = value; + } else if (std.mem.eql(u8, key, "name")) { + result.name = value; + } + // Unknown keys are ignored. + } + + return result; + } + }; + /// Sanitize the ID according to the spec: /// /// Valid ids must include only characters from the set: [a-zA-Z0-9-_+.]. @@ -167,7 +197,8 @@ pub const Metadata = struct { fn decodeValue(alloc: Allocator, value: []const u8, max_len: usize) error{ OutOfMemory, Overflow, - Invalid, + InvalidBase64, + InvalidUtf8, }![]const u8 { const Encoder = std.base64.standard.Encoder; @@ -180,10 +211,10 @@ pub const Metadata = struct { const decoded = simd.base64.decode( value, buf, - ) catch return error.Invalid; + ) catch return error.InvalidBase64; // Must be valid UTF-8 - if (!std.unicode.utf8ValidateSlice(decoded)) return error.Invalid; + if (!std.unicode.utf8ValidateSlice(decoded)) return error.InvalidUtf8; if (decoded.len > max_len) return error.Overflow; return decoded; } @@ -215,6 +246,10 @@ pub const Payload = struct { alloc.free(self.buf); } + pub fn isValidUtf8(self: *const Payload) bool { + return std.unicode.utf8ValidateSlice(self.data); + } + /// Iterate the whitespace-separated MIME types of the payload. /// Matches Python str.split() used by kitty. pub fn mimeIterator(self: *const Payload) std.mem.TokenIterator(u8, .any) { @@ -335,12 +370,19 @@ test "metadata: invalid mime base64 dropped" { try testing.expect((try Metadata.parse(arena.allocator(), "type=wdata:mime=!!!")) == null); } -test "metadata: invalid mime utf8 dropped" { +test "metadata: invalid mime utf8 reported" { const testing = std.testing; var arena: std.heap.ArenaAllocator = .init(testing.allocator); defer arena.deinit(); // base64 of 0xff 0xfe - try testing.expect((try Metadata.parse(arena.allocator(), "type=wdata:mime=//4=")) == null); + try testing.expectError( + error.InvalidValue, + Metadata.parse(arena.allocator(), "type=wdata:mime=//4="), + ); + try testing.expectEqual( + Operation.wdata, + Metadata.operation("type=wdata:mime=//4=").?, + ); } test "metadata: pw and name" { @@ -353,7 +395,7 @@ test "metadata: pw and name" { try testing.expectEqualStrings("app", meta.name); } -test "metadata: over-long name dropped" { +test "metadata: over-long name reported" { const testing = std.testing; var arena: std.heap.ArenaAllocator = .init(testing.allocator); defer arena.deinit(); @@ -364,7 +406,10 @@ test "metadata: over-long name dropped" { "type=read:name=", Encoder.encode(&buf, long), }); - try testing.expect((try Metadata.parse(arena.allocator(), raw)) == null); + try testing.expectError( + error.InvalidValue, + Metadata.parse(arena.allocator(), raw), + ); } test "metadata: empty name" { @@ -407,3 +452,11 @@ test "payload: invalid base64" { Payload.init(testing.allocator, "!!!"), ); } + +test "payload: decoded text must be valid utf8" { + const testing = std.testing; + // Valid base64 encoding of a single 0xff byte. + const payload = try Payload.init(testing.allocator, "/w=="); + defer payload.deinit(testing.allocator); + try testing.expect(!payload.isValidUtf8()); +} diff --git a/src/terminal/kitty/clipboard_write.zig b/src/terminal/kitty/clipboard_write.zig index 7834ed631..b080b1fa8 100644 --- a/src/terminal/kitty/clipboard_write.zig +++ b/src/terminal/kitty/clipboard_write.zig @@ -243,6 +243,7 @@ pub const WriteState = struct { assert(meta.mime.len > 0); const decoded = try Payload.init(alloc, payload); defer decoded.deinit(alloc); + if (!decoded.isValidUtf8()) return error.Invalid; var it = decoded.mimeIterator(); // Copy the target only if at least one valid alias exists. @@ -496,6 +497,22 @@ test "write: aliases resolve at commit" { try testing.expectEqualStrings("Ghostty", committed.contents[2].data); } +test "write: alias payload must be valid utf8" { + const testing = std.testing; + const alloc = testing.allocator; + + const begin_meta: Metadata = .{ .op = .write }; + var state: WriteState = try .init(alloc, &begin_meta, .{}); + defer state.deinit(alloc); + + const alias_meta: Metadata = .{ .op = .walias, .mime = "text/plain" }; + // Valid base64 encoding of a single 0xff byte. + try testing.expectError( + error.Invalid, + state.alias(alloc, &alias_meta, "/w=="), + ); +} + test "write: default limit when unset" { const testing = std.testing; const alloc = testing.allocator; diff --git a/src/terminal/stream_terminal.zig b/src/terminal/stream_terminal.zig index 3b3cf7eb2..589f09f6e 100644 --- a/src/terminal/stream_terminal.zig +++ b/src/terminal/stream_terminal.zig @@ -784,10 +784,24 @@ pub const Handler = struct { // Decode and validate the metadata. var arena: std.heap.ArenaAllocator = .init(self.terminal.gpa()); defer arena.deinit(); - const meta = (try kitty_clipboard.Metadata.parse( + const meta = (kitty_clipboard.Metadata.parse( arena.allocator(), v.metadata, - )) orelse return; + ) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.InvalidValue => { + const state = self.kitty_clipboard_write orelse return; + switch (kitty_clipboard.Metadata.operation(v.metadata) orelse return) { + .wdata, .walias => self.kittyClipboardFinish( + state, + .EINVAL, + v.terminator, + ), + .read, .write => {}, + } + return; + }, + }) orelse return; const payload = v.payload orelse ""; switch (meta.op) { @@ -815,6 +829,7 @@ pub const Handler = struct { error.Invalid => return, }; defer decoded.deinit(alloc); + if (!decoded.isValidUtf8()) return; // Without a clipboard_read effect nothing can serve the read. // EPERM is the protocol's denial so clients degrade gracefully. @@ -1069,10 +1084,15 @@ pub const Handler = struct { payload: []const u8, terminator: osc.Terminator, ) error{OutOfMemory}!void { - // Aliases without a transaction or without a target MIME type - // are silently ignored. + // Aliases without a transaction are silently ignored. Once a + // transaction exists, a missing target MIME type is invalid and + // aborts the transaction. const state = self.kitty_clipboard_write orelse return; - if (meta.mime.len == 0) return; + if (meta.mime.len == 0) return self.kittyClipboardFinish( + state, + .EINVAL, + terminator, + ); state.alias( self.terminal.gpa(), @@ -4236,7 +4256,7 @@ test "kitty clipboard new write replaces in-flight transaction" { ); } -test "kitty clipboard invalid walias payload aborts with EINVAL" { +test "kitty clipboard invalid write packets abort with EINVAL" { var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); defer t.deinit(testing.allocator); @@ -4249,20 +4269,66 @@ test "kitty clipboard invalid walias payload aborts with EINVAL" { var s: Stream = .init(.{ .allocator = testing.allocator, .handler = handler }); defer s.deinit(); + const invalid_packets = [_][]const u8{ + // Alias payload decodes to a non-UTF-8 byte. + "\x1B]5522;type=walias:mime=dGV4dC9wbGFpbg==;/w==\x1B\\", + // Alias has no target MIME type. + "\x1B]5522;type=walias;VEVYVA==\x1B\\", + // Alias target MIME decodes to non-UTF-8 bytes. + "\x1B]5522;type=walias:mime=//4=;VEVYVA==\x1B\\", + // Write data MIME decodes to non-UTF-8 bytes. + "\x1B]5522;type=wdata:mime=//4=;R2hvc3Q=\x1B\\", + }; + + for (invalid_packets) |packet| { + S.responses_len = 0; + s.nextSlice("\x1B]5522;type=write:id=w\x1B\\"); + s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;R2hvc3Q=\x1B\\"); + s.nextSlice(packet); + try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + S.responseSlice(), + ); + try testing.expect(!s.handler.semantic_failure); + + // The transaction is gone: a commit does nothing further. + s.nextSlice("\x1B]5522;type=wdata\x1B\\"); + try testing.expectEqual(@as(usize, 0), S.write_count); + try testing.expectEqualStrings( + "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + S.responseSlice(), + ); + } +} + +test "kitty clipboard invalid read text does not abort a write" { + var t: Terminal = try .init(testing.io, testing.allocator, .{ .cols = 80, .rows = 24 }); + defer t.deinit(testing.allocator); + + const S = KittyClipboardCapture; + S.reset(); + S.read_result = .{ .success = .{} }; + + var handler: Handler = .init(&t); + handler.effects.write_pty = &S.writePty; + handler.effects.clipboard_read = &S.clipboardRead; + handler.effects.clipboard_write = &S.clipboardWrite; + var s: Stream = .init(.{ .allocator = testing.allocator, .handler = handler }); + defer s.deinit(); + s.nextSlice("\x1B]5522;type=write:id=w\x1B\\"); s.nextSlice("\x1B]5522;type=wdata:mime=dGV4dC9wbGFpbg==;R2hvc3Q=\x1B\\"); - s.nextSlice("\x1B]5522;type=walias:mime=dGV4dC9wbGFpbg==;!!!\x1B\\"); - try testing.expectEqualStrings( - "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", - S.responseSlice(), - ); - try testing.expect(!s.handler.semantic_failure); - // The transaction is gone: a commit does nothing further. + // The read payload decodes to a non-UTF-8 byte. It is dropped without + // invoking the clipboard effect or disturbing the write transaction. + s.nextSlice("\x1B]5522;type=read;/w==\x1B\\"); + try testing.expectEqual(@as(usize, 0), S.read_count); + try testing.expectEqual(@as(usize, 0), S.responses_len); + s.nextSlice("\x1B]5522;type=wdata\x1B\\"); - try testing.expectEqual(@as(usize, 0), S.write_count); + try testing.expectEqual(@as(usize, 1), S.write_count); try testing.expectEqualStrings( - "\x1B]5522;type=write:status=EINVAL:id=w\x1B\\", + "\x1B]5522;type=write:status=DONE:id=w\x1B\\", S.responseSlice(), ); } diff --git a/src/termio/stream_handler.zig b/src/termio/stream_handler.zig index 356ef275e..85b5057f6 100644 --- a/src/termio/stream_handler.zig +++ b/src/termio/stream_handler.zig @@ -1026,14 +1026,29 @@ pub const StreamHandler = struct { ) error{ OutOfMemory, WriteFailed }!void { const kitty_clipboard = terminal.kitty.clipboard; - // Decode and validate the metadata. Malformed metadata drops - // the packet without any response, matching kitty. + // Decode and validate the metadata. Malformed structure drops + // the packet without a response. Invalid decoded text on a write + // data or alias packet aborts an in-flight transaction. var arena: std.heap.ArenaAllocator = .init(self.alloc); defer arena.deinit(); - const meta = (try kitty_clipboard.Metadata.parse( + const meta = (kitty_clipboard.Metadata.parse( arena.allocator(), v.metadata, - )) orelse return; + ) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.InvalidValue => { + const state = self.kitty_clipboard_write orelse return; + switch (kitty_clipboard.Metadata.operation(v.metadata) orelse return) { + .wdata, .walias => try self.kittyClipboardWriteFinish( + state, + .EINVAL, + v.terminator, + ), + .read, .write => {}, + } + return; + }, + }) orelse return; switch (meta.op) { .read => try self.kittyClipboardRead( @@ -1089,6 +1104,10 @@ pub const StreamHandler = struct { return; }, }; + if (!decoded.isValidUtf8()) { + arena.deinit(); + return; + } // The targets type ('.') asks for the listing of available // types rather than data. Requested types beyond the cap are @@ -1235,10 +1254,15 @@ pub const StreamHandler = struct { payload: []const u8, terminator: terminal.osc.Terminator, ) error{ OutOfMemory, WriteFailed }!void { - // Aliases without a transaction or without a target MIME type - // are silently ignored, matching kitty. + // Aliases without a transaction are silently ignored. Once a + // transaction exists, a missing target MIME type is invalid and + // aborts the transaction. const state = self.kitty_clipboard_write orelse return; - if (meta.mime.len == 0) return; + if (meta.mime.len == 0) return self.kittyClipboardWriteFinish( + state, + .EINVAL, + terminator, + ); state.alias( self.alloc,