Subject: Re: bug#70988: (read FUNCTION) uses Latin-1 [PATCH]

From: Pip Cet <[email-masked]>
Date: Thu, 13 Feb 2025 14:08:02 +0000

"Stefan Kangas" <[email-masked]> writes:

> Mattias Engdegård <[email-masked]> writes:
>
>> After looking further into the Lisp reader/printer I found two more
>> silent Latin-1 assumptions. In all three cases, I firmly believe the
>> following to be true:
>>
>> * The behaviour is not intended but just code accidents.
>> * They should hardly affect any user code at all.
>> * They are nevertheless clear bugs which should be fixed.
>>
>> Further on this will have to wait until after Emacs 30 has been branched to avoid delaying that more important task.
>
> FWIW, the proposed patch looks like a bug fix to me as well, so I think
> we should install it.

Here's a patch which avoids a few crashes in lread.c.  If we change the
behavior of readcharfuns' return values to always be treated as
multibyte, but fail to verify that its return value satisfies
CHARACTERP, as proposed, further problems will arise.

In the event that there is interest in fixing Emacs not to crash in
these circumstances, let me know and I can commit this.

Pip

>From e9d48abeb199db7d76386639adadba5c7e45177c Mon Sep 17 00:00:00 2001
From: Pip Cet <[email-masked]>
Subject: [PATCH] Avoid crashes in lread.c when invalid characters are read

* src/lread.c (readchar): Don't crash for non-fixnum return values.
(read_filtered_event): Don't crash for invalid symbol properties.
(Fread_char):
(Fread_char_exclusive):
(character_name_to_code): Check 'FIXNUMP' before using 'XFIXNUM'.
(read_char_escape): Use 'invalid_syntax' rather than crashing for
invalid characters.
---
 src/lread.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/lread.c b/src/lread.c
index 6af95873bb8..5875b489c97 100644
--- a/src/lread.c
+++ b/src/lread.c
@@ -398,7 +398,7 @@ readchar (Lisp_Object readcharfun, bool *multibyte)
 
   tem = call0 (readcharfun);
 
-  if (NILP (tem))
+  if (!FIXNUMP (tem))
     return -1;
   return XFIXNUM (tem);
 
@@ -816,7 +816,7 @@ read_filtered_event (bool no_switch_frame, bool ascii_required,
 	      tem1 = Fget (Fcar (tem), Qascii_character);
 	      /* Merge this symbol's modifier bits
 		 with the ASCII equivalent of its basic code.  */
-	      if (!NILP (tem1))
+	      if (FIXNUMP (tem1) && FIXNUMP (Fcar (Fcdr (tem))))
 		XSETFASTINT (val, XFIXNUM (tem1) | XFIXNUM (Fcar (Fcdr (tem))));
 	    }
 	}
@@ -898,7 +898,7 @@ DEFUN ("read-char", Fread_char, Sread_char, 0, 3, 0,
     }
   val = read_filtered_event (1, 1, 1, ! NILP (inherit_input_method), seconds);
 
-  return (NILP (val) ? Qnil
+  return (!FIXNUMP (val) ? Qnil
 	  : make_fixnum (char_resolve_modifier_mask (XFIXNUM (val))));
 }
 
@@ -976,7 +976,7 @@ DEFUN ("read-char-exclusive", Fread_char_exclusive, Sread_char_exclusive, 0, 3,
 
   val = read_filtered_event (1, 1, 0, ! NILP (inherit_input_method), seconds);
 
-  return (NILP (val) ? Qnil
+  return (!FIXNUMP (val) ? Qnil
 	  : make_fixnum (char_resolve_modifier_mask (XFIXNUM (val))));
 }
 
@@ -2820,7 +2820,7 @@ character_name_to_code (char const *name, ptrdiff_t name_len,
       invalid_syntax_lisp (CALLN (Fformat, format, namestr), readcharfun);
     }
 
-  return XFIXNUM (code);
+  return FIXNUMP (code) ? XFIXNUM (code) : -1;
 }
 
 /* Bound on the length of a Unicode character name.  As of
@@ -3058,7 +3058,8 @@ read_char_escape (Lisp_Object readcharfun, int next_char)
       chr = c;
       break;
     }
-  eassert (chr >= 0 && chr < (1 << CHARACTERBITS));
+  if (chr < 0 || chr > (1 << CHARACTERBITS))
+    invalid_syntax ("Invalid character", readcharfun);
 
   /* Apply Control modifiers, using the rules:
      \C-X = ascii_ctrl(nomod(X)) | mods(X)  if nomod(X) is one of:
-- 
2.48.1