[PATCH] avoid a few strncpy-induced buffer overruns

[PATCH] avoid a few strncpy-induced buffer overruns

[Jim Meyering]

I've been on a little crusade to avoid strncpy abuse
and spotted/fixed a few uses here.

[ Since each of the two buffers in procfs.c is NUL-filled via its decl,
  instead of adding a NUL-terminating statement after each strncpy, I could
  have changed the 3rd parameter in each strncpy call to be one smaller, but
  that would not be as clear as the explicit NUL termination, and would be
  fragile enough that a comment explaining the trickiness would be required,
  to dissuade anyone who might be tempted to change the initialization. ]


From dc611a67de35b54dcd00c9a298279474f06f42c3 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@redhat.com>
Date: Tue, 24 Apr 2012 19:59:49 +0200
Subject: [PATCH] avoid a few strncpy-induced buffer overruns

* procfs.c (procfs_make_note_section): Be sure to NUL-terminate
fname and psargs before trying to concatenate.
* tui/tui-stack.c (tui_get_function_from_frame): NUL-terminate
"name" before applying strchr.
---
 gdb/ChangeLog       |    8 ++++++++
 gdb/procfs.c        |    5 +++--
 gdb/tui/tui-stack.c |    1 +
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 5b2b105..fa7ae78 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2012-04-24  Jim Meyering  <meyering@redhat.com>
+
+	avoid a few strncpy-induced buffer overruns
+	* procfs.c (procfs_make_note_section): Be sure to NUL-terminate
+	fname and psargs before trying to concatenate.
+	* tui/tui-stack.c (tui_get_function_from_frame): NUL-terminate
+	"name" before applying strchr.
+
 2012-04-24  Yao Qi  <yao@codesourcery.com>

 	Revert this patch to allow breakpoint always-inserted
diff --git a/gdb/procfs.c b/gdb/procfs.c
index cb4bc7c..d7c2946 100644
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -5725,8 +5725,9 @@ procfs_make_note_section (bfd *obfd, int *note_size)
   if (get_exec_file (0))
     {
       strncpy (fname, lbasename (get_exec_file (0)), sizeof (fname));
-      strncpy (psargs, get_exec_file (0),
-	       sizeof (psargs));
+      fname[sizeof (fname) - 1] = 0;
+      strncpy (psargs, get_exec_file (0), sizeof (psargs));
+      psargs[sizeof (psargs) - 1] = 0;

       inf_args = get_inferior_args ();
       if (inf_args && *inf_args &&
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index ef50a98..262a6bf 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -228,6 +228,7 @@ tui_get_function_from_frame (struct frame_info *fi)
   if (*p == '<')
     p++;
   strncpy (name, p, sizeof (name) - 1);
+  name[sizeof (name) - 1] = 0;
   p = strchr (name, '(');
   if (!p)
     p = strchr (name, '>');
--
1.7.10.281.g8271e

Re: [PATCH] avoid a few strncpy-induced buffer overruns

[Tom Tromey]

>>>>> "Jim" == Jim Meyering <jim@meyering.net> writes:

Jim> I've been on a little crusade to avoid strncpy abuse
Jim> and spotted/fixed a few uses here.

This is ok.  Thanks.

Tom

Re: [PATCH] avoid a few strncpy-induced buffer overruns

[Jim Meyering]

Tom Tromey wrote:
>>>>>> "Jim" == Jim Meyering <jim@meyering.net> writes:
>
> Jim> I've been on a little crusade to avoid strncpy abuse
> Jim> and spotted/fixed a few uses here.
>
> This is ok.  Thanks.

Thanks for the quick review.
I've cvs-committed that.

cvs feels so clunky and archaic, now...

Re: [PATCH] avoid a few strncpy-induced buffer overruns

[Joel Brobecker]

> cvs feels so clunky and archaic, now...

I use git-cvsexportcommit for that, now. The only CVS command
I need to run befre commit is "cvs update" of my CVS repo.

-- 
Joel

Re: [PATCH] avoid a few strncpy-induced buffer overruns

[Jim Meyering]

Joel Brobecker wrote:
>> cvs feels so clunky and archaic, now...
>
> I use git-cvsexportcommit for that, now. The only CVS command
> I need to run befre commit is "cvs update" of my CVS repo.

Thanks.  So do I.
However, I made the mistake of running "cvs diff" to be sure that
those were the only modified files ;-)  Felt like it'd never finish.