* bug#11374: emacsclient: avoid invalid strcpy upon partial send and buffer overrun upon failed send.
@ 2012-04-28 22:01 Jim Meyering
2012-04-29 6:51 ` bug#11374: corrected patch: now, actually compiles Jim Meyering
0 siblings, 1 reply; 3+ messages in thread
From: Jim Meyering @ 2012-04-28 22:01 UTC (permalink / raw)
To: 11374
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@redhat.com>
Date: Sat, 28 Apr 2012 22:43:55 +0200
2012-04-28 Jim Meyering <meyering@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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* bug#11374: corrected patch: now, actually compiles
2012-04-28 22:01 bug#11374: emacsclient: avoid invalid strcpy upon partial send and buffer overrun upon failed send Jim Meyering
@ 2012-04-29 6:51 ` Jim Meyering
2012-05-02 10:41 ` Chong Yidong
0 siblings, 1 reply; 3+ messages in thread
From: Jim Meyering @ 2012-04-29 6:51 UTC (permalink / raw)
To: 11374
The first version lacked a definition of min and misspelled memmove.
From 3b96ff1a4f8825d3942c56c79069ca5ffea7d4f6 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@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))
+
\f
/* 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
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-05-02 10:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-28 22:01 bug#11374: emacsclient: avoid invalid strcpy upon partial send and buffer overrun upon failed send Jim Meyering
2012-04-29 6:51 ` bug#11374: corrected patch: now, actually compiles Jim Meyering
2012-05-02 10:41 ` Chong Yidong
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).