GNU bug report logs - #77325
Crash in Fjson_parse_buffer: ZV changes underneath it?

Previous Next

Full log

View this message in rfc822 format

From: Pip Cet <pipcet <at> protonmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: dancol <at> dancol.org, 77325 <at> debbugs.gnu.org
Subject: bug#77325: Crash in Fjson_parse_buffer: ZV changes underneath it?
Date: Sun, 30 Mar 2025 10:40:42 +0000

"Eli Zaretskii" <eliz <at> gnu.org> writes:

>> Date: Sat, 29 Mar 2025 15:37:17 +0000
>> From: Pip Cet <pipcet <at> protonmail.com>
>> Cc: dancol <at> dancol.org, 77325 <at> debbugs.gnu.org
>>
>> The code does seem a bit complicated for what it's trying to achieve, to
>> be honest. I think it'd be clearer just to write:
>>
>>   unsigned char *begin = PT_ADDR;
>>   unsigned char *end = min (GPT_ADDR, ZV_ADDR);
>>   unsigned char *secondary_begin = min (GAP_END_ADDR, ZV_ADDR));
>>   unsigned char *secondary_end = ZV_ADDR;
>>
>>   json_parser_init (&p, conf, begin, end, secondary_begin,
>> 		    secondary_end);
>>
>> json_parser_init fixes up secondary_begin and secondary_end to be NULL
>> pointers in this case.
>
> I'd prefer the secondary_* values to be set NULL explicitly, otherwise
> it is hard to understand what the code does without looking deep into
> the subroutines of json_parse.

Sorry, it took me a while.  I was surprised by how tricky this is, and I
admit I was confused briefly because ZV == GPT doesn't make ZV_ADDR ==
GPT_ADDR.  I can see that most places in the code manipulate byte
positions, not pointers, and I think I understand the reason now :-)

From dd30d60a7992da0cef4e09a8a7e8989249dab4f1 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet <at> protonmail.com>
Subject: [PATCH] Respect narrowed buffers when parsing JSON (bug#77325)

* src/json.c (Fjson_insert): Simplify 'memcpy' argument.
(Fjson_parse_buffer): Only read to ZV, not all the way to Z.
* test/src/json-tests.el (with-all-gap-positions-in-temp-buffer):
New macro.
(json-parse-buffer/restricted): New test.
---
 src/json.c             | 13 +++++++------
 test/src/json-tests.el | 26 ++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/src/json.c b/src/json.c
index f438d191bde..5795c582ce0 100644
--- a/src/json.c
+++ b/src/json.c
@@ -641,7 +641,7 @@ DEFUN ("json-insert", Fjson_insert, Sjson_insert, 1, MANY,
   move_gap_both (PT, PT_BYTE);
   if (GAP_SIZE < jo.size)
     make_gap (jo.size - GAP_SIZE);
-  memcpy ((char *) BEG_ADDR + PT_BYTE - BEG_BYTE, jo.buf, jo.size);
+  memcpy (GPT_ADDR, jo.buf, jo.size);
 
   /* No need to keep allocation beyond this point.  */
   unbind_to (count, Qnil);
@@ -1754,15 +1754,16 @@ DEFUN ("json-parse-buffer", Fjson_parse_buffer, Sjson_parse_buffer,
 
   struct json_parser p;
   unsigned char *begin = PT_ADDR;
-  unsigned char *end = GPT_ADDR;
+  unsigned char *end = (GPT == ZV) ? GPT_ADDR : ZV_ADDR;
   unsigned char *secondary_begin = NULL;
   unsigned char *secondary_end = NULL;
-  if (GPT_ADDR < Z_ADDR)
+  if (PT == ZV)
+    begin = end = NULL;
+  else if (GPT > PT && GPT < ZV && GAP_SIZE > 0)
     {
+      end = GPT_ADDR;
       secondary_begin = GAP_END_ADDR;
-      if (secondary_begin < PT_ADDR)
-	secondary_begin = PT_ADDR;
-      secondary_end = Z_ADDR;
+      secondary_end = ZV_ADDR;
     }
 
   json_parser_init (&p, conf, begin, end, secondary_begin,
diff --git a/test/src/json-tests.el b/test/src/json-tests.el
index 94b6cfcffca..1cb667ddeac 100644
--- a/test/src/json-tests.el
+++ b/test/src/json-tests.el
@@ -315,6 +315,32 @@ json-parse-buffer/trailing
     (should-not (bobp))
     (should (looking-at-p (rx " [456]" eos)))))
 
+(defmacro with-all-gap-positions-in-temp-buffer (string &rest body)
+  "Create a temporary buffer containing STRING, and evaluate BODY
+with each possible gap position.
+See also `with-temp-buffer'."
+  `(with-temp-buffer
+     (insert ,string)
+     (dotimes (i (- (point-max) (point-min)))
+       (goto-char (- (point-max) i))
+       (insert "X")
+       (delete-region (1- (point)) (point))
+       ,@body)))
+
+(ert-deftest json-parse-buffer/restricted ()
+  (with-all-gap-positions-in-temp-buffer
+   "[123] [456] [789]"
+   (pcase-dolist (`((,beg . ,end) ,result)
+                  '(((7 . 12) [456])
+                    ((1 . 6) [123])
+                    ((13 . 18) [789])))
+     (goto-char beg)
+     (narrow-to-region beg end)
+     (should (equal (json-parse-buffer) result))
+     (should (= (point) end))
+     (should-error (json-parse-buffer) :type 'json-end-of-file)
+     (widen))))
+
 (ert-deftest json-parse-with-custom-null-and-false-objects ()
   (let* ((input
           "{ \"abc\" : [9, false] , \"def\" : null }")
-- 
2.48.1

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.