CERBERUS/kitty
1
0
Fork 0
mirror of https://github.com/kovidgoyal/kitty.git synced 2026-09-06 09:48:27 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

Graphics: Make reading images robust against clients that truncate files that are being read by kitty

Browse source
This commit is contained in:
Kovid Goyal 2026-08-23 17:05:06 +05:30
parent 8eee836555
commit 7616cfe854
No known key found for this signature in database
GPG key ID: 06BC317B515ACE7C
4 changed files with 91 additions and 43 deletions
Download patch file Download diff file Expand all files Collapse all files

8
docs/changelog.rst
View file

@ -264,6 +264,14 @@ Detailed list of changes
- Text sizing protocol: Fix a buffer overflow when a natural width (no explicit ``w`` key) text sizing escape code contains a grapheme cluster longer than four codepoints
- Graphics protocol: Fix a crash when transmitting image data via a file or
shared memory object (``t=f``, ``t=t`` or ``t=s``) and the client truncates
it while kitty is reading from it.
- Graphics protocol: Fix reading image data from a file or shared memory
object at an offset (the ``O`` key) failing unless the offset happened to be
a multiple of the system page size
0.48.2 [2026-07-30]
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

88
kitty/graphics.c
View file

@ -111,9 +111,6 @@ free_load_data(LoadData *ld) {
ld->buf_used = 0;
ld->buf_capacity = 0;
ld->buf = NULL;
if (ld->mapped_file) munmap(ld->mapped_file, ld->mapped_file_sz);
ld->mapped_file = NULL;
ld->mapped_file_sz = 0;
ld->loading_for = (const ImageAndFrame){0};
}
@ -328,19 +325,42 @@ set_command_failed_response(const char *code, const char *fmt, ...) {
}
static bool
mmap_img_file(GraphicsManager *self, int fd, size_t sz, off_t offset) {
if (!sz) {
struct stat s;
if (fstat(fd, &s) != 0) ABRT(EBADF, "Failed to fstat() the fd: %d file with error: [%d] %s", fd, errno, strerror(errno));
sz = s.st_size;
read_img_file(GraphicsManager *self, int fd, size_t sz, off_t offset, size_t max_to_read) {
LoadData *ld = &self->currently_loading;
struct stat s;
if (fstat(fd, &s) != 0) ABRT(EBADF, "Failed to fstat() the fd: %d file with error: [%d] %s", fd, errno, strerror(errno));
// The graphics protocol specification mandates that only regular files be
// read. Reading from FIFOs/devices/etc. can block forever or have
// side-effects.
if (!S_ISREG(s.st_mode)) ABRT(EBADF, "The image file with fd: %d is not a regular file", fd);
if (!sz) sz = offset < s.st_size ? (size_t)(s.st_size - offset) : 0;
if (sz > max_to_read) sz = max_to_read;
free(ld->buf);
// Note that the data is copied rather than mmap()ed as the client is free
// to truncate the file at any time, which would cause SIGBUS when reading
// from a mapping of it. With read() a truncated file simply results in a
// short read, which is reported as insufficient data by the caller.
ld->buf = malloc(sz ? sz : 1);
if (!ld->buf) ABRT(ENOMEM, "Out of memory allocating %zu bytes to read image file", sz);
ld->buf_capacity = sz;
ld->buf_used = 0;
while (ld->buf_used < sz) {
ssize_t n = pread(fd, ld->buf + ld->buf_used, sz - ld->buf_used, offset + (off_t)ld->buf_used);
if (n < 0) {
if (errno == EINTR) continue;
ABRT(EIO, "Failed to read from image file fd: %d with error: [%d] %s", fd, errno, strerror(errno));
}
if (!n) break; // EOF, the file is smaller than expected, reported as insufficient data by the caller
ld->buf_used += n;
}
void *addr = mmap(0, sz, PROT_READ, MAP_SHARED, fd, offset);
if (addr == MAP_FAILED)
ABRT(EBADF, "Failed to map image file fd: %d at offset: %zd with size: %zu with error: [%d] %s", fd, offset, sz, errno, strerror(errno));
self->currently_loading.mapped_file = addr;
self->currently_loading.mapped_file_sz = sz;
return true;
err:
// Discard any partially read data, the caller is not guaranteed to call
// free_load_data() on failure.
free(ld->buf);
ld->buf = NULL;
ld->buf_capacity = 0;
ld->buf_used = 0;
return false;
}
@ -635,7 +655,11 @@ load_image_data(
ABRT("EPERM", "Permission denied to read image file");
}
}
load_data->loading_completed_successfully = mmap_img_file(self, fd, g->data_sz, g->data_offset);
// When the data needs further processing the entire (possibly
// compressed) payload is needed, otherwise reading more than the
// expected number of bytes is pointless.
const size_t max_to_read = (g->compressed || data_fmt == PNG) ? MAX_DATA_SZ : load_data->data_sz;
load_data->loading_completed_successfully = read_img_file(self, fd, g->data_sz, g->data_offset, max_to_read);
safe_close(fd, __FILE__, __LINE__);
if (transmission_type == 't' && strstr(fname, "tty-graphics-protocol") != NULL) {
if (global_state.boss) {
@ -650,20 +674,15 @@ load_image_data(
}
static Image *
process_image_data(GraphicsManager *self, Image *img, const GraphicsCommand *g, const unsigned char transmission_type, const uint32_t data_fmt) {
process_image_data(GraphicsManager *self, Image *img, const GraphicsCommand *g, const uint32_t data_fmt) {
bool needs_processing = g->compressed || data_fmt == PNG;
if (needs_processing) {
uint8_t *buf;
size_t bufsz;
#define IB \
{ \
if (self->currently_loading.buf) { \
buf = self->currently_loading.buf; \
bufsz = self->currently_loading.buf_used; \
} else { \
buf = self->currently_loading.mapped_file; \
bufsz = self->currently_loading.mapped_file_sz; \
} \
#define IB \
{ \
buf = self->currently_loading.buf; \
bufsz = self->currently_loading.buf_used; \
}
switch (g->compressed) {
case 'z':
@ -691,21 +710,10 @@ process_image_data(GraphicsManager *self, Image *img, const GraphicsCommand *g,
if (self->currently_loading.buf_used < self->currently_loading.data_sz) {
ABRT("ENODATA", "Insufficient image data: %zu < %zu", self->currently_loading.buf_used, self->currently_loading.data_sz);
}
if (self->currently_loading.mapped_file) {
munmap(self->currently_loading.mapped_file, self->currently_loading.mapped_file_sz);
self->currently_loading.mapped_file = NULL;
self->currently_loading.mapped_file_sz = 0;
}
} else {
if (transmission_type == 'd') {
if (self->currently_loading.buf_used < self->currently_loading.data_sz) {
ABRT("ENODATA", "Insufficient image data: %zu < %zu", self->currently_loading.buf_used, self->currently_loading.data_sz);
} else self->currently_loading.data = self->currently_loading.buf;
} else {
if (self->currently_loading.mapped_file_sz < self->currently_loading.data_sz) {
ABRT("ENODATA", "Insufficient image data: %zu < %zu", self->currently_loading.mapped_file_sz, self->currently_loading.data_sz);
} else self->currently_loading.data = self->currently_loading.mapped_file;
}
if (self->currently_loading.buf_used < self->currently_loading.data_sz) {
ABRT("ENODATA", "Insufficient image data: %zu < %zu", self->currently_loading.buf_used, self->currently_loading.data_sz);
} else self->currently_loading.data = self->currently_loading.buf;
self->currently_loading.loading_completed_successfully = true;
}
return img;
@ -819,7 +827,7 @@ handle_add_command(GraphicsManager *self, const GraphicsCommand *g, const uint8_
img = load_image_data(self, img, g, tt, fmt, payload);
if (!img || !self->currently_loading.loading_completed_successfully) return NULL;
self->currently_loading.loading_for = (const ImageAndFrame){0};
img = process_image_data(self, img, g, tt, fmt);
img = process_image_data(self, img, g, fmt);
if (!img) return NULL;
size_t required_sz = (size_t)(self->currently_loading.is_opaque ? 3 : 4) * self->currently_loading.width * self->currently_loading.height;
if (self->currently_loading.data_sz != required_sz)
@ -1752,7 +1760,7 @@ handle_animation_frame_load_command(GraphicsManager *self, GraphicsCommand *g, I
img = load_image_data(self, img, g, tt, fmt, payload);
if (!img || !load_data->loading_completed_successfully) return NULL;
self->currently_loading.loading_for = (const ImageAndFrame){0};
img = process_image_data(self, img, g, tt, fmt);
img = process_image_data(self, img, g, fmt);
if (!img || !load_data->loading_completed_successfully) return img;
const unsigned long bytes_per_pixel = load_data->is_opaque ? 3 : 4;

3
kitty/graphics.h
View file

@ -135,9 +135,6 @@ typedef struct {
uint8_t *buf;
size_t buf_capacity, buf_used;
uint8_t *mapped_file;
size_t mapped_file_sz;
size_t data_sz;
uint8_t *data;
bool is_4byte_aligned;

35
kitty_tests/graphics.py
View file

@ -463,6 +463,41 @@ class TestGraphics(BaseTest):
s.reset()
self.assertEqual(g.disk_cache.total_size, 0)
def test_load_images_from_file_edge_cases(self):
s, g, pl, sl = load_helpers(self)
random_data = byte_block(32 * 1024)
with tempfile.NamedTemporaryFile(prefix='tty-graphics-protocol-') as f:
# A window of the file specified with a non page aligned offset
f.write(b'x' * 3 + random_data + b'y' * 5), f.flush()
sl(f.name, s=1024, v=8, t='f', S=len(random_data), O=3, expecting_data=random_data)
# A file that is truncated after the size declared in the command
# must be reported as insufficient data rather than crashing
f.seek(0), f.truncate(), f.write(random_data[:128]), f.flush()
self.ae(pl(f.name, s=1024, v=8, t='f', S=len(random_data)), f'ENODATA:Insufficient image data: 128 < {len(random_data)}')
# Ditto when the size is not declared and is read from the file itself
self.ae(pl(f.name, s=1024, v=8, t='f'), f'ENODATA:Insufficient image data: 128 < {len(random_data)}')
# An offset past the end of the file
self.ae(pl(f.name, s=1024, v=8, t='f', O=4096), f'ENODATA:Insufficient image data: 0 < {len(random_data)}')
# Only regular files may be read
with tempfile.TemporaryDirectory(prefix='tty-graphics-protocol-') as tdir:
fifo = os.path.join(tdir, 'fifo')
os.mkfifo(fifo)
self.assertTrue(pl(fifo, s=1024, v=8, t='f').startswith('EBADF:'), 'Reading from a FIFO was not refused')
# A shared memory object truncated to less than the declared size
name = '/kitty-test-shm-truncated'
shm_write(name, random_data[:64])
self.ae(pl(name, s=1024, v=8, t='s', S=len(random_data)), f'ENODATA:Insufficient image data: 64 < {len(random_data)}')
self.assertRaises(FileNotFoundError, shm_unlink, name) # check that the object was deleted
s.reset()
self.assertEqual(g.disk_cache.total_size, 0)
@unittest.skipIf(Image is None, 'PIL not available, skipping PNG tests')
def test_load_png(self):
s, g, pl, sl = load_helpers(self)