bug#3552: 23.0.94; backward-prefix-chars: Point before start of properties

[Noam Postavsky <npostavs@users.sourceforge.net>]

# bumping severity due to crash potential
severity 3352 important
tag 3352 + patch
quit

On Thu, Jun 2, 2016 at 11:34 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> Still a problem with latest Emacs 25 pretest, and on Windows 8, Emacs
> 25.0.94 this actually crashes Emacs too.

Running under valgrind I get "invalid read of size 1" in
Fbackward_prefix_chars on GNU/Linux as well (see below). I think this
is a long standing bug that allows reading from before beginning of
the buffer. It was introduced way back in 1998, 1fd3172dd4819
"(Fbackward_prefix_chars): Set point properly while scanning."

diff --git a/src/syntax.c b/src/syntax.c
index 4ac1c8d..0235767 100644
--- a/src/syntax.c
+++ b/src/syntax.c
@@ -2174,12 +2174,16 @@ DEFUN ("backward-prefix-chars",
Fbackward_prefix_chars, Sbackward_prefix_chars,

   DEC_BOTH (pos, pos_byte);

-  while (pos + 1 > beg && !char_quoted (pos, pos_byte)
+  while (!char_quoted (pos, pos_byte)
      /* Previous statement updates syntax table.  */
      && ((c = FETCH_CHAR (pos_byte), SYNTAX (c) == Squote)
          || SYNTAX_PREFIX (c)))
     {
-      DEC_BOTH (pos, pos_byte);
+      opoint = pos;
+      opoint_byte = pos_byte;
+
+      if (pos + 1 > beg)
+    DEC_BOTH (pos, pos_byte);
     }

   SET_PT_BOTH (opoint, opoint_byte);


The (pos + 1 > beg) check originally followed the decrementing of pos,
but after that commit the check came before (and also doesn't end the
loop anymore). Therefore, if (pos == beg), we decrement and then try
to look at the syntax of the character at position (beg-1). This may
segfault, or trigger the "point before start of properties" error in
update_interval (eventually called from char_quoted).

I propose the following patch be applied to the emacs-25 branch:

@@ -3109,8 +3109,9 @@ DEFUN ("backward-prefix-chars",
Fbackward_prefix_chars, Sbackward_prefix_chars,
       opoint = pos;
       opoint_byte = pos_byte;

-      if (pos + 1 > beg)
-    DEC_BOTH (pos, pos_byte);
+      DEC_BOTH (pos, pos_byte);
+      if (pos < beg)
+        break;
     }

   SET_PT_BOTH (opoint, opoint_byte);


This fixes the originally reported error, and the invalid read, cf the
valgrind output mentioned above:

==2557== Invalid read of size 1
==2557==    at 0x56691D: Fbackward_prefix_chars (syntax.c:3113)
==2557==    by 0x541543: Ffuncall (eval.c:2690)
==2557==    by 0x5704D9: exec_byte_code (bytecode.c:880)
==2557==    by 0x541151: funcall_lambda (eval.c:2855)
==2557==    by 0x54167E: Ffuncall (eval.c:2742)
==2557==    by 0x5704D9: exec_byte_code (bytecode.c:880)
==2557==    by 0x541151: funcall_lambda (eval.c:2855)
==2557==    by 0x54167E: Ffuncall (eval.c:2742)
==2557==    by 0x53D941: Ffuncall_interactively (callint.c:252)
==2557==    by 0x5414E2: Ffuncall (eval.c:2673)
==2557==    by 0x53F07D: Fcall_interactively (callint.c:840)
==2557==    by 0x54157F: Ffuncall (eval.c:2700)
==2557==  Address 0x146aab9f is 1 bytes before a block of size 2,146 alloc'd
==2557==    at 0x4C2CB1D: realloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2557==    by 0x527F90: lrealloc (alloc.c:1427)
==2557==    by 0x529628: xrealloc (alloc.c:856)
==2557==    by 0x4F837F: enlarge_buffer_text (buffer.c:4974)
==2557==    by 0x4FB610: make_gap_larger (insdel.c:393)
==2557==    by 0x4FB6D7: make_gap (insdel.c:491)
==2557==    by 0x4FC5D7: insert_from_string_1 (insdel.c:926)
==2557==    by 0x4FD157: insert_from_string (insdel.c:872)
==2557==    by 0x535103: general_insert_function (editfns.c:2468)
==2557==    by 0x53514C: Finsert (editfns.c:2504)
==2557==    by 0x571D28: exec_byte_code (bytecode.c:1509)
==2557==    by 0x541151: funcall_lambda (eval.c:2855)