GNU bug report logs -
#11374
emacsclient: avoid invalid strcpy upon partial send and buffer overrun upon failed send.
Previous Next
Reported by: Jim Meyering <jim <at> meyering.net>
Date: Sat, 28 Apr 2012 22:03:02 UTC
Severity: normal
Done: Chong Yidong <cyd <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 11374 in the body.
You can then email your comments to 11374 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#11374
; Package
emacs
.
(Sat, 28 Apr 2012 22:03:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Jim Meyering <jim <at> meyering.net>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sat, 28 Apr 2012 22:03:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
This patch is untested. I didn't even try to compile it.
From dde8915c64f020e72a141702257db91fafca5523 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sat, 28 Apr 2012 22:43:55 +0200
2012-04-28 Jim Meyering <meyering <at> redhat.com>
Subject: [PATCH 2/5] emacsclient: avoid invalid strcpy upon partial send and
buffer overrun
...upon failed send.
* lib-src/emacsclient.c (send_to_emacs): Simplify and fix two bugs:
- before, we could call strcpy with overlapping buffers upon partial
send, but strcpy cannot handle overlapping buffers. Use memcopy.
- before, we would call strcpy(send_buffer, &send_buffer[-1]) upon
failed "send", resulting in an invalid read and a buffer overrun
when that first byte is not 0. Diagnose the failure.
Also, call strlen just once, rather than for each iteration.
---
lib-src/emacsclient.c | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 48b4384..addc089 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -783,33 +783,33 @@ sock_err_message (const char *function_name)
static void
send_to_emacs (HSOCKET s, const char *data)
{
- while (data)
+ if (!data)
+ return;
+
+ size_t dlen = strlen (data);
+ while (*data)
{
- size_t dlen = strlen (data);
- if (dlen + sblen >= SEND_BUFFER_SIZE)
- {
- int part = SEND_BUFFER_SIZE - sblen;
- strncpy (&send_buffer[sblen], data, part);
- data += part;
- sblen = SEND_BUFFER_SIZE;
- }
- else if (dlen)
- {
- strcpy (&send_buffer[sblen], data);
- data = NULL;
- sblen += dlen;
- }
- else
- break;
+ size_t part = min (dlen, SEND_BUFFER_SIZE - sblen);
+ memcpy (&send_buffer[sblen], data, part);
+ data += part;
+ sblen += part;
if (sblen == SEND_BUFFER_SIZE
|| (sblen > 0 && send_buffer[sblen-1] == '\n'))
{
int sent = send (s, send_buffer, sblen, 0);
+ if (sent < 0)
+ {
+ message (TRUE, "%s: failed to send %d bytes to socket: %s\n",
+ progname, sblen, strerror (errno));
+ fail ();
+ }
if (sent != sblen)
- strcpy (send_buffer, &send_buffer[sent]);
+ memcopy (send_buffer, &send_buffer[sent], sblen - sent);
sblen -= sent;
}
+
+ dlen -= part;
}
}
--
1.7.10.382.g62bc8
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#11374
; Package
emacs
.
(Sun, 29 Apr 2012 06:53:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 11374 <at> debbugs.gnu.org (full text, mbox):
The first version lacked a definition of min and misspelled memmove.
From 3b96ff1a4f8825d3942c56c79069ca5ffea7d4f6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering <at> redhat.com>
Date: Sat, 28 Apr 2012 22:43:55 +0200
Subject: [PATCH 2/5] emacsclient: avoid invalid strcpy upon partial send and
buffer overrun
...upon failed send.
* lib-src/emacsclient.c (min): Define.
(send_to_emacs): Simplify and fix two bugs:
- before, we could call strcpy with overlapping buffers upon partial
send, but strcpy cannot handle overlapping buffers. Use memmove.
- before, we would call strcpy(send_buffer, &send_buffer[-1]) upon
failed "send", resulting in an invalid read and a buffer overrun
when that first byte is not 0. Diagnose the failure.
Also, call strlen just once, rather than for each iteration.
---
lib-src/emacsclient.c | 38 ++++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/lib-src/emacsclient.c b/lib-src/emacsclient.c
index 48b4384..0ca13fa 100644
--- a/lib-src/emacsclient.c
+++ b/lib-src/emacsclient.c
@@ -119,6 +119,8 @@ char *(getcwd) (char *, size_t);
# define IF_LINT(Code) /* empty */
#endif
+#define min(x, y) (((x) < (y)) ? (x) : (y))
+
/* Name used to invoke this program. */
const char *progname;
@@ -783,33 +785,33 @@ sock_err_message (const char *function_name)
static void
send_to_emacs (HSOCKET s, const char *data)
{
- while (data)
+ if (!data)
+ return;
+
+ size_t dlen = strlen (data);
+ while (*data)
{
- size_t dlen = strlen (data);
- if (dlen + sblen >= SEND_BUFFER_SIZE)
- {
- int part = SEND_BUFFER_SIZE - sblen;
- strncpy (&send_buffer[sblen], data, part);
- data += part;
- sblen = SEND_BUFFER_SIZE;
- }
- else if (dlen)
- {
- strcpy (&send_buffer[sblen], data);
- data = NULL;
- sblen += dlen;
- }
- else
- break;
+ size_t part = min (dlen, SEND_BUFFER_SIZE - sblen);
+ memcpy (&send_buffer[sblen], data, part);
+ data += part;
+ sblen += part;
if (sblen == SEND_BUFFER_SIZE
|| (sblen > 0 && send_buffer[sblen-1] == '\n'))
{
int sent = send (s, send_buffer, sblen, 0);
+ if (sent < 0)
+ {
+ message (TRUE, "%s: failed to send %d bytes to socket: %s\n",
+ progname, sblen, strerror (errno));
+ fail ();
+ }
if (sent != sblen)
- strcpy (send_buffer, &send_buffer[sent]);
+ memmove (send_buffer, &send_buffer[sent], sblen - sent);
sblen -= sent;
}
+
+ dlen -= part;
}
}
--
1.7.10.382.g62bc8
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#11374
; Package
emacs
.
(Wed, 02 May 2012 10:44:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 11374 <at> debbugs.gnu.org (full text, mbox):
Jim Meyering <jim <at> meyering.net> writes:
> The first version lacked a definition of min and misspelled memmove.
>
> From 3b96ff1a4f8825d3942c56c79069ca5ffea7d4f6 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering <at> redhat.com>
> Date: Sat, 28 Apr 2012 22:43:55 +0200
> Subject: [PATCH 2/5] emacsclient: avoid invalid strcpy upon partial send and
> buffer overrun
Thanks, committed.
bug closed, send any further explanations to
11374 <at> debbugs.gnu.org and Jim Meyering <jim <at> meyering.net>
Request was from
Chong Yidong <cyd <at> gnu.org>
to
control <at> debbugs.gnu.org
.
(Wed, 02 May 2012 10:44:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Wed, 30 May 2012 11:24:03 GMT)
Full text and
rfc822 format available.
This bug report was last modified 11 years and 330 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.