Refuse to read image files in /proc /sys and /dev

Also, open the image file with O_NONBLOCK to ensure we dont hang opening
a FIFO

docs/graphics-protocol.rst

@@ -284,7 +284,7 @@ and can take the values:
 Value of `t`          Meaning
 ==================    ============
 ``d``                 Direct (the data is transmitted within the escape code itself)
-``f``                 A simple file (regular files only, not named pipes or similar)
+``f``                 A simple file (regular files only, not named pipes, device files, etc.)
 ``t``                 A temporary file, the terminal emulator will delete the file after reading the pixel data. For security reasons
                       the terminal emulator should only delete the file if it
                       is in a known temporary directory, such as :file:`/tmp`,
@@ -302,7 +302,11 @@ Value of `t`          Meaning
 
 When opening files, the terminal emulator must follow symlinks. In case of
 symlink loops or too many symlinks, it should fail and respond with an error,
-similar to reporting any other kind of I/O error.
+similar to reporting any other kind of I/O error. Since the file paths come
+from potentially untrusted sources, terminal emulators **must** refuse to read
+any device/socket/etc. special files. Only regular files are allowed.
+Additionally, terminal emulators may refuse to read files in *sensitive*
+parts of the filesystem, such as :file:`/proc`, :file:`/sys`, :file:`/dev/`, etc.
 
 Local client
 ^^^^^^^^^^^^^^

kitty/boss.py

@@ -131,6 +131,7 @@ from .utils import (
     func_name,
     get_editor,
     get_new_os_window_size,
+    is_ok_to_read_image_file,
     is_path_in_temp_dir,
     less_version,
     log_error,
@@ -2441,6 +2442,9 @@ class Boss:
             with suppress(FileNotFoundError):
                 os.remove(path)
 
+    def is_ok_to_read_image_file(self, path: str, fd: int) -> bool:
+        return is_ok_to_read_image_file(path, fd)
+
     def set_update_check_process(self, process: Optional['PopenType[bytes]'] = None) -> None:
         if self.update_check_process is not None:
             with suppress(Exception):

kitty/graphics.c

@@ -421,8 +421,19 @@ load_image_data(GraphicsManager *self, Image *img, const GraphicsCommand *g, con
             if (g->payload_sz > 2048) ABRT("EINVAL", "Filename too long");
             snprintf(fname, sizeof(fname)/sizeof(fname[0]), "%.*s", (int)g->payload_sz, payload);
             if (transmission_type == 's') fd = safe_shm_open(fname, O_RDONLY, 0);
-            else fd = safe_open(fname, O_CLOEXEC | O_RDONLY, 0);
+            else fd = safe_open(fname, O_CLOEXEC | O_RDONLY | O_NONBLOCK, 0);  // O_NONBLOCK so that opening a FIFO pipe does not block
             if (fd == -1) ABRT("EBADF", "Failed to open file for graphics transmission with error: [%d] %s", errno, strerror(errno));
+            if (global_state.boss) {
+                DECREF_AFTER_FUNCTION PyObject *cret_ = PyObject_CallMethod(global_state.boss, "is_ok_to_read_image_file", "si", fname, fd);
+                if (cret_ == NULL) {
+                    PyErr_Print();
+                    ABRT("EBADF", "Failed to check file for read permission");
+                }
+                if (cret_ != Py_True) {
+                    log_error("Refusing to read image file as permission was denied");
+                    ABRT("EPERM", "Permission denied to read image file");
+                }
+            }
             load_data->loading_completed_successfully = mmap_img_file(self, fd, g->data_sz, g->data_offset);
             safe_close(fd, __FILE__, __LINE__);
             if (transmission_type == 't' && strstr(fname, "tty-graphics-protocol") != NULL) {

kitty/utils.py

@@ -712,6 +712,26 @@ def is_path_in_temp_dir(path: str) -> bool:
     return False
 
 
+def is_ok_to_read_image_file(path: str, fd: int) -> bool:
+    import stat
+    path = os.path.abspath(os.path.realpath(path))
+    try:
+        path_stat = os.stat(path, follow_symlinks=True)
+        fd_stat = os.fstat(fd)
+    except OSError:
+        return False
+    if not os.path.samestat(path_stat, fd_stat):
+        return False
+    parts = path.split(os.sep)[1:]
+    if len(parts) < 1:
+        return False
+    if parts[0] in ('sys', 'proc', 'dev'):
+        if parts[0] == 'dev':
+            return len(parts) > 2 and parts[1] == 'shm'
+        return False
+    return stat.S_ISREG(fd_stat.st_mode)
+
+
 def resolve_abs_or_config_path(path: str, env: Optional[Mapping[str, str]] = None, conf_dir: Optional[str] = None) -> str:
     path = os.path.expanduser(path)
     path = expandvars(path, env or {})

kitty_tests/datatypes.py

@@ -20,7 +20,7 @@ from kitty.fast_data_types import (
 )
 from kitty.fast_data_types import Cursor as C
 from kitty.rgb import to_color
-from kitty.utils import is_path_in_temp_dir, sanitize_title, sanitize_url_for_dispay_to_user
+from kitty.utils import is_ok_to_read_image_file, is_path_in_temp_dir, sanitize_title, sanitize_url_for_dispay_to_user
 
 from . import BaseTest, filled_cursor, filled_history_buf, filled_line_buf
 
@@ -444,6 +444,21 @@ class TestDataTypes(BaseTest):
                 self.assertTrue(is_path_in_temp_dir(os.path.join(prefix, path)))
         for path in ('/home/xy/d.png', '/tmp/../home/x.jpg'):
             self.assertFalse(is_path_in_temp_dir(os.path.join(path)))
+        for path in '/proc/self/cmdline /dev/tty'.split():
+            if os.path.exists(path):
+                with open(path) as pf:
+                    self.assertFalse(is_ok_to_read_image_file(path, pf.fileno()), path)
+        fifo = os.path.join(tempfile.gettempdir(), 'test-kitty-fifo')
+        os.mkfifo(fifo)
+        fifo_fd = os.open(fifo, os.O_RDONLY | os.O_NONBLOCK)
+        try:
+            self.assertFalse(is_ok_to_read_image_file(fifo, fifo_fd), fifo)
+        finally:
+            os.close(fifo_fd)
+            os.remove(fifo)
+        if os.path.isdir('/dev/shm'):
+            with tempfile.NamedTemporaryFile(dir='/dev/shm') as tf:
+                self.assertTrue(is_ok_to_read_image_file(tf.name, tf.fileno()), fifo)
         self.ae(sanitize_url_for_dispay_to_user(
             'h://a\u0430b.com/El%20Ni%C3%B1o/'), 'h://xn--ab-7kc.com/El Niño/')