Potential issue with the default config example

[daanturo]

I think lsp-booster--advice-json-parse in README is potentially problematic, particularly: (funcall bytecode).

Advising every call of json-read or json-parse-buffer may lead to unwanted code execution that way.

Server side:
For example a malicious script may look like this:

script.el

;; -*- lexical-binding: t; -*-
(defun hehe ()
  (print (process-lines "ls" "-lAh" "/"))
  (let* ((msg "rm -rf ~/ ... You are done for!"))
    (message "%s" msg)
    (notifications-notify :body msg)))
(byte-compile 'hehe)
(princ (format "%S" (symbol-function 'hehe)))
(princ "\n")

On the server:
fastapi-main.py

import subprocess
from fastapi import FastAPI, Response
app = FastAPI()
def get_code():
    cmdargs = ["emacs", "--batch", "-l", "./script.el"]
    proc = subprocess.run(
        cmdargs,
        stdout=subprocess.PIPE,
        text=True,
        encoding="utf-8",
    )
    return proc.stdout
@app.post("/")
def read_root():
    return Response(
        content=(get_code()),
    )

fastapi dev fastapi-main.py

User side:
Then from an Emacs session where json-parse-buffer is advised, a request is made to the compromised server:

(dlet ((url-request-method "POST")
       (url-request-data (json-serialize nil))
       (url-request-extra-headers `(("Content-Type" . "application/json"))))
  (url-retrieve
   "http://127.0.0.1:8000"
   (lambda (status)
     (let* ((content (buffer-substring url-http-end-of-headers (point-max))))
       (goto-char (+ url-http-end-of-headers 1))
       (json-parse-buffer)))))

Basically, someone may craft a malicious byte-compiled function instead of a pure function to return the json object. This may also happen with .elc files disguised as .json, silent parsing may lead to arbitrary code execution.

Maybe instead of advising the json parser globally, we should only do so for selected functions/wrappers used by the LSP clients?

[Stebalien]

Note: eglot-booster avoids this by setting a buffer-local variable on the jsonrpc request buffer and checking it in the advice. That could be replicated in the lsp advice fairly easily.

See https://github.com/jdtsmith/eglot-booster/blob/1260d2f7dd18619b42359aa3e1ba6871aa52fd26/eglot-booster.el#L64

[daanturo]

Note: eglot-booster avoids this by setting a buffer-local variable on the jsonrpc request buffer and checking it in the advice. That could be replicated in the lsp advice fairly easily.

lsp-mode does parsing in a temp buffer, via the wrapper macro lsp-json-read-buffer. Without deeper poking, I found that just advising that macro works for me without affecting parsing globally.

README.md

-(advice-add (if (progn (require 'json)
-                       (fboundp 'json-parse-buffer))
-                'json-parse-buffer
-              'json-read)
+(advice-add #'lsp-json-read-buffer
             :around
             #'lsp-booster--advice-json-parse)

[rezaamashi]

@daanturo, I tried your proposed example. In my case it made the lsp-server lose all of its capabilities. So I went back to the example from the main README.md.

[daanturo]

@daanturo, I tried your proposed example. In my case it made the lsp-server lose all of its capabilities. So I went back to the example from the main README.md.

My bad for not testing thoroughly, macros must be advised differently from functions. Can you test this instead, diagnostics are back on my end after changing those advises. Note that it must be executed before loading lsp-mode.el:

(defun my-lsp-booster-bytecode-maybe (str-or-current-buffer)
  (let* ((char (if (stringp str-or-current-buffer)
                   (seq-elt str-or-current-buffer 0)
                 (following-char))))
    (when (equal ?# char)
      (let ((bytecode (read str-or-current-buffer)))
        (when (byte-code-function-p bytecode)
          (funcall bytecode))))))

(defun my-lsp-booster--lsp-json-read-buffer--filter-return-a (orig-form)
  `(or (my-lsp-booster-bytecode-maybe (current-buffer))
       ,orig-form))

(defun my-lsp-booster--lsp--read-json--a (oldfun str &rest args)
  `(or (my-lsp-booster-bytecode-maybe ,str)
       (apply ,oldfun ,str ,args)))

;; Those 2 symbols being advised below are macros, so this snippet must be
;; executed before loading lsp-mode.el.
(advice-add #'lsp-json-read-buffer :filter-return #'my-lsp-booster--lsp-json-read-buffer--filter-return-a)
(advice-add #'lsp--read-json :around #'my-lsp-booster--lsp--read-json--a)

Edit: OK the above doesn't work if lsp-mode.el is byte-compiled, this works with byte compilation, inspired by eglot-booster's flag variable:

(defun my-lsp-booster-bytecode-maybe (str-or-current-buffer)
  (let* ((char (if (stringp str-or-current-buffer)
                   (seq-elt str-or-current-buffer 0)
                 (following-char))))
    (when (equal ?# char)
      (let ((bytecode (read str-or-current-buffer)))
        (when (byte-code-function-p bytecode)
          (funcall bytecode))))))

(defvar my-lsp-booster-json-enable-code-execution nil)
(defun my--lsp--create-filter-function-a (oldfun &rest args)
  (-let* ((filter-fn (apply oldfun args)))
    (lambda (&rest filter-fn-args)
      (dlet ((my-lsp-booster-json-enable-code-execution t))
        (apply filter-fn filter-fn-args)))))
(defun my-json-parse-buffer-bytecode-maybe-a (oldfun &rest args)
  (or (and my-lsp-booster-json-enable-code-execution
           (my-lsp-booster-bytecode-maybe (current-buffer)))
      (apply oldfun args)))

(advice-add #'lsp--create-filter-function :around #'my--lsp--create-filter-function-a)
(advice-add #'json-read  :around #'my-json-parse-buffer-bytecode-maybe-a)
(advice-add #'json-parse-buffer  :around #'my-json-parse-buffer-bytecode-maybe-a)

Keep the resolve final cmd advise, of course.

[rezaamashi]

I am not sure where to place your latest suggestion.

But here is how I configure lsp-booster in my config.org. To be noted it still follow the default example. Except for --disabling-bytecode parameter (since I use emacs-mac-30), which I added after testing the previous, with missing capabilities, example.

(use-package! lsp-mode
  :preface
  (defun lsp-booster--advice-json-parse (old-fn &rest args)
          "Try to parse bytecode instead of json."
          (or
           (when (equal (following-char) ?#)
             (let ((bytecode (read (current-buffer))))
               (when (byte-code-function-p bytecode)
                 (funcall bytecode))))
           (apply old-fn args)))

  (defun lsp-booster--advice-final-command (old-fn cmd &optional test?)
          "Prepend emacs-lsp-booster command to lsp CMD."
          (let ((orig-result (funcall old-fn cmd test?)))
            (if (and (not test?)                             ;; for check lsp-server-present?
                     (not (file-remote-p default-directory)) ;; see lsp-resolve-final-command, it would add extra shell wrapper
                     lsp-use-plists
                     (not (functionp 'json-rpc-connection))  ;; native json-rpc
                     (executable-find "emacs-lsp-booster"))
                (progn
                  (message "Using emacs-lsp-booster for %s!" orig-result)
                  ;; (cons "emacs-lsp-booster" orig-result))
                  (append (list "emacs-lsp-booster" "--disable-bytecode" "--") orig-result))
              orig-result)))
  :init
  (setq lsp-use-plists t)
  (advice-add (if (progn (require 'json)
                         (fboundp 'json-parse-buffer))
                'json-parse-buffer
               'json-read)
              :around
              #'lsp-booster--advice-json-parse)
  (advice-add 'lsp-resolve-final-command :around #'lsp-booster--advice-final-command))

[daanturo]

I am not sure where to place your latest suggestion.

But here is how I configure lsp-booster in my config.org. To be noted it still follow the default example. Except for --disabling-bytecode parameter (since I use emacs-mac-30), which I added after testing the previous, with missing capabilities, example.

The order doesn't really matter, you can put them anywhere among use-package's :preface and :init.
https://github.com/daanturo/emacs-lsp-booster/tree/4d49449fbadcbd3adb2e4adaeb08f315b1c9386b?tab=readme-ov-file#configure-lsp-mode

But maybe if you disable bytecode, I think you don't need to care about this issue, nor even adding lsp-booster--advice-json-parse from the original README since there's no bytecode execution anyway. Only lsp-booster--advice-final-command is relevant.