groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[RFC] input.cpp: Remove use of strncat(3)


From: Alejandro Colomar
Subject: [RFC] input.cpp: Remove use of strncat(3)
Date: Mon, 5 Dec 2022 23:39:37 +0100

See the strncat(3) Linux manual page for details about why strncat(3)
is actively harmful.

Still, we don't test for truncation, which is a sign that strlcpy(3)
(or strncpy(3)) before it were incorrectly used, but that should be
addressed in a separate patch.  Of course, strncpy(3) wasn't tested
for truncation because it simply can't report it.

One could argue that since we allocate enough size, we never truncate;
however, with that argument, we could simply call strcpy(3), and say
it's "safe".  I differ with such arguments, on the basis that the lines
of code are then uncoupled, and modifications in one of them might
break code at a different location.

Moreover, guaranteeing that one location is safe for strcpy(3) can be
easy, but guaranteeing that all calls to strcpy(3) are safe is not
maintainable.  Since we're not in performance-critical software, it's
simpler to just add those checks, just for being safe.  Even in such
high-performance software, checks should be done, although other
functions and data structures need to be used to keep the performance.

Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

Hi Branden,

This patch would need some hooks in the build system to depend on libbsd
if libc doesn't provide strlcpy(3).  We could do something like what
Guillem Jover did for shadow:
<https://github.com/shadow-maint/shadow/pull/573/files>

Cheers,

Alex

 src/roff/troff/input.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/roff/troff/input.cpp b/src/roff/troff/input.cpp
index ade608563..344de8b0d 100644
--- a/src/roff/troff/input.cpp
+++ b/src/roff/troff/input.cpp
@@ -7892,7 +7892,7 @@ void do_macro_source(bool quietly)
                        MACRO_POSTFIX, sizeof(MACRO_POSTFIX) - 1) == 0) {
          char *s = new char[fnlen + sizeof(MACRO_PREFIX)];
          strcpy(s, MACRO_PREFIX);
-         strncat(s, fn, fnlen - sizeof(MACRO_POSTFIX) + 1);
+          strlcat(s, fn, sizeof(MACRO_PREFIX));
          fp = mac_path->open_file(s, &path);
          delete[] s;
        }
-- 
2.38.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]