Subject: [PATCH] bindat (strz): Write null terminator after variable length string
From: Richard Hansen <[email-masked]>
To: [email-masked]
Date: Mon, 13 Jun 2022 17:48:15 -0400
List-Id: <debbugs-submit.debbugs.gnu.org>

X-Debbugs-CC: [email-masked]

Attached patch:

* lisp/emacs-lisp/bindat.el (bindat--pack-strz): Explicitly write a
null byte after packing a variable-length string to ensure proper
termination when packing to a pre-allocated string.
* doc/lispref/processes.texi (Bindat Types): Update documentation.
* test/lisp/emacs-lisp/bindat-tests.el (bindat-test--str-strz-prealloc):
Update tests.

From 29e40414f1b344774ed9085a8f125fd0801276c3 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Thu, 9 Jun 2022 20:41:50 -0400
Subject: [PATCH] bindat (strz): Write null terminator after variable length
 string

* lisp/emacs-lisp/bindat.el (bindat--pack-strz): Explicitly write a
null byte after packing a variable-length string to ensure proper
termination when packing to a pre-allocated string.
* doc/lispref/processes.texi (Bindat Types): Update documentation.
* test/lisp/emacs-lisp/bindat-tests.el (bindat-test--str-strz-prealloc):
Update tests.
---
 doc/lispref/processes.texi           | 17 ++++-------------
 lisp/emacs-lisp/bindat.el            |  3 +++
 test/lisp/emacs-lisp/bindat-tests.el |  4 ++--
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index 8c8f8fd6b2..179980c0ed 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -3495,24 +3495,15 @@ Bindat Types
 @item strz &optional @var{len}
 If @var{len} is not provided: Variable-length null-terminated unibyte
 string (@pxref{Text Representations}).  When packing, the entire input
-string is copied to the packed output.  The following byte will be
-null (zero) unless a pre-allocated string was provided to
-@code{bindat-pack}, in which case that byte is left unmodified.  The
-length of the packed output is the length of the input string plus one
-(for the null terminator).  The input string must not contain any null
-bytes.  If the input string is multibyte with only ASCII and
+string is copied to the packed output followed by a null (zero) byte.
+The length of the packed output is the length of the input string plus
+one (for the null terminator).  The input string must not contain any
+null bytes.  If the input string is multibyte with only ASCII and
 @code{eight-bit} characters, it is converted to unibyte before it is
 packed; other multibyte strings signal an error.  When unpacking, the
 resulting string contains all bytes up to (but excluding) the null
 byte.
 
-@quotation Caution
-If a pre-allocated string is provided to @code{bindat-pack}, the
-packed output will not be properly null-terminated unless the
-pre-allocated string already has a null byte at the appropriate
-location.
-@end quotation
-
 If @var{len} is provided: @code{strz} behaves the same as @code{str}
 with one difference: When unpacking, the first null byte encountered
 in the packed string and all subsequent bytes are excluded from the
diff --git a/lisp/emacs-lisp/bindat.el b/lisp/emacs-lisp/bindat.el
index 9ba89a5e3f..46e2a4901c 100644
--- a/lisp/emacs-lisp/bindat.el
+++ b/lisp/emacs-lisp/bindat.el
@@ -450,6 +450,9 @@ bindat--pack-strz
         ;; need to scan the input string looking for a null byte.
         (error "Null byte encountered in input strz string"))
       (aset bindat-raw (+ bindat-idx i) (aref v i)))
+    ;; Explicitly write a null terminator in case the user provided a
+    ;; pre-allocated string to bindat-pack that wasn't zeroed first.
+    (aset bindat-raw (+ bindat-idx len) 0)
     (setq bindat-idx (+ bindat-idx len 1))))
 
 (defun bindat--pack-bits (len v)
diff --git a/test/lisp/emacs-lisp/bindat-tests.el b/test/lisp/emacs-lisp/bindat-tests.el
index 7d1233ded7..cc223ad14e 100644
--- a/test/lisp/emacs-lisp/bindat-tests.el
+++ b/test/lisp/emacs-lisp/bindat-tests.el
@@ -182,8 +182,8 @@ bindat-test--str-strz-prealloc
                 ((((x strz 2)) ((x . "a"))) . "ax")
                 ((((x strz 2)) ((x . "ab"))) . "ab")
                 ((((x strz 2)) ((x . "abc"))) . "ab")
-                ((,(bindat-type strz) "") . "xx")
-                ((,(bindat-type strz) "a") . "ax")))
+                ((,(bindat-type strz) "") . "\0x")
+                ((,(bindat-type strz) "a") . "a\0")))
     (let ((prealloc (make-string 2 ?x)))
       (apply #'bindat-pack (append (car tc) (list prealloc)))
       (should (equal prealloc (cdr tc))))))
-- 
2.36.1