INN commit: trunk (5 files)
INN Commit
rra at isc.org
Sun Sep 4 12:52:31 UTC 2016
Date: Sunday, September 4, 2016 @ 05:52:31
Author: iulius
Revision: 10062
Improve the robustness of snprintf handling
snprintf() returns -1 on error, and *value larger than supplied buffer
size* if formatted string will not fit in supplied buffer.
If you add/subtract snprintf() return value without validating its
range, this will lead up to disaster.
Thanks to Yuriy M. Kaminskiy for the patch.
Modified:
trunk/CONTRIBUTORS
trunk/innfeed/imap_connection.c
trunk/lib/setproctitle.c
trunk/lib/timer.c
trunk/nnrpd/nnrpd.c
---------------------------+
CONTRIBUTORS | 2 -
innfeed/imap_connection.c | 29 ++++++++++++++++++++--
lib/setproctitle.c | 6 +++-
lib/timer.c | 56 ++++++++++++++++++++++++++++++++++++--------
nnrpd/nnrpd.c | 4 +++
5 files changed, 82 insertions(+), 15 deletions(-)
Modified: CONTRIBUTORS
===================================================================
--- CONTRIBUTORS 2016-09-04 12:48:39 UTC (rev 10061)
+++ CONTRIBUTORS 2016-09-04 12:52:31 UTC (rev 10062)
@@ -277,4 +277,4 @@
Dennis Preiser, Paolo Amoroso, Dennis Davis, River Tarnell, Jochen Schmitt,
Tim Fardell, Remco Rijnders, David Binderman, Tony Evans, Christian Garbs,
Jesse Rehmer, Colin Watson, Lauri Tirkkonen, Christian Mock, Marcus Jodorf,
-Richard Kettlewell
+Richard Kettlewell, Yuriy M. Kaminskiy
Modified: innfeed/imap_connection.c
===================================================================
--- innfeed/imap_connection.c 2016-09-04 12:48:39 UTC (rev 10061)
+++ innfeed/imap_connection.c 2016-09-04 12:52:31 UTC (rev 10062)
@@ -3636,6 +3636,7 @@
int size = strlen(*out);
int fsize = size;
int newsize = size + 9+strlen(deliver_rcpt_to)+newrcptlen+3;
+ int rc;
char c;
/* see if we need to grow the string */
@@ -3650,8 +3651,15 @@
c = newrcpt[newrcptlen];
newrcpt[newrcptlen] = '\0';
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
- size += snprintf((*out) + size, newsize - size, deliver_rcpt_to, newrcpt);
+ rc = snprintf((*out) + size, newsize - size, deliver_rcpt_to, newrcpt);
#pragma GCC diagnostic warning "-Wformat-nonliteral"
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if (rc >= newsize - size) {
+ size = newsize;
+ } else {
+ size += rc;
+ }
newrcpt[newrcptlen] = c;
strlcpy((*out) + size, ">\r\n", newsize - size);
@@ -3714,6 +3722,7 @@
{
int size = strlen(*out);
int newsize = size + strlen(sep)+1+strlen(deliver_to_header)+newrcptlen+1;
+ int rc;
char c;
/* see if we need to grow the string */
@@ -3722,13 +3731,27 @@
(*out) = xrealloc(*out, *outalloc);
}
- size += snprintf((*out) + size, newsize - size, "%s<", sep);
+ rc = snprintf((*out) + size, newsize - size, "%s<", sep);
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if (rc >= newsize - size) {
+ size = newsize;
+ } else {
+ size += rc;
+ }
c = newrcpt[newrcptlen];
newrcpt[newrcptlen] = '\0';
#pragma GCC diagnostic ignored "-Wformat-nonliteral"
- size += snprintf((*out) + size, newsize - size, deliver_to_header,newrcpt);
+ rc = snprintf((*out) + size, newsize - size, deliver_to_header,newrcpt);
#pragma GCC diagnostic warning "-Wformat-nonliteral"
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if (rc >= newsize - size) {
+ size = newsize;
+ } else {
+ size += rc;
+ }
newrcpt[newrcptlen] = c;
strlcpy((*out) + size, ">", newsize - size);
Modified: lib/setproctitle.c
===================================================================
--- lib/setproctitle.c 2016-09-04 12:48:39 UTC (rev 10061)
+++ lib/setproctitle.c 2016-09-04 12:52:31 UTC (rev 10062)
@@ -35,6 +35,8 @@
delta = snprintf(title, sizeof(title), "%s: ", message_program_name);
if (delta < 0)
delta = 0;
+ else if ((size_t)delta >= sizeof(title))
+ delta = sizeof(title);
}
va_start(args, format);
vsnprintf(title + delta, sizeof(title) - delta, format, args);
@@ -82,7 +84,7 @@
message_program_name if it's set. */
if (message_program_name != NULL) {
delta = snprintf(title, length, "%s: ", message_program_name);
- if (delta < 0 || (size_t) delta > length)
+ if (delta < 0 || (size_t) delta >= length)
return;
if (delta > 0) {
title += delta;
@@ -92,7 +94,7 @@
va_start(args, format);
delta = vsnprintf(title, length, format, args);
va_end(args);
- if (delta < 0 || (size_t) delta > length)
+ if (delta < 0 || (size_t) delta >= length)
return;
if (delta > 0) {
title += delta;
Modified: lib/timer.c
===================================================================
--- lib/timer.c 2016-09-04 12:48:39 UTC (rev 10061)
+++ lib/timer.c 2016-09-04 12:52:31 UTC (rev 10062)
@@ -322,17 +322,35 @@
{
struct timer *node;
size_t off = 0;
+ int rc;
/* This results in "child/parent nn(nn)" instead of the arguably more
intuitive "parent/child" but it's easy. Since we ensure sane snprintf
semantics, it's safe to defer checking for overflow until after
formatting all of the timer data. */
- for (node = timer; node != NULL; node = node->parent)
- off += snprintf(buf + off, len - off, "%s/",
+ for (node = timer; node != NULL; node = node->parent) {
+ rc = snprintf(buf + off, len - off, "%s/",
TMRlabel(labels, node->id));
- off--;
- off += snprintf(buf + off, len - off, " %lu(%lu) ", timer->total,
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if ((size_t)rc >= len - off) {
+ off = len;
+ } else {
+ off += rc;
+ }
+ }
+ if (off > 0)
+ off--;
+
+ rc = snprintf(buf + off, len - off, " %lu(%lu) ", timer->total,
timer->count);
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if ((size_t)rc >= len - off) {
+ off = len;
+ } else {
+ off += rc;
+ }
if (off == len) {
warn("timer log too long while processing %s",
TMRlabel(labels, timer->id));
@@ -359,6 +377,7 @@
char *buf;
unsigned int i;
size_t len, off;
+ int rc;
/* To find the needed buffer size, note that a 64-bit unsigned number can
be up to 20 digits long, so each timer can be 52 characters. We also
@@ -369,14 +388,33 @@
buffer isn't large enough it will just result in logged errors. */
len = 52 * timer_count + 27 + (prefix == NULL ? 0 : strlen(prefix)) + 1;
buf = xmalloc(len);
+ off = 0;
if (prefix == NULL)
- off = 0;
+ rc = 0;
else
- off = snprintf(buf, len, "%s ", prefix);
- off += snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
- for (i = 0; i < timer_count; i++)
- if (timers[i] != NULL)
+ rc = snprintf(buf, len, "%s ", prefix);
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if ((size_t)rc >= len) {
+ off = len;
+ } else {
+ off += rc;
+ }
+
+ rc = snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
+ if (rc < 0) {
+ /* Do nothing. */
+ } else if ((size_t)rc >= len - off) {
+ off = len;
+ } else {
+ off += rc;
+ }
+
+ for (i = 0; i < timer_count; i++) {
+ if (timers[i] != NULL) {
off += TMRsumone(labels, timers[i], buf + off, len - off);
+ }
+ }
notice("%s", buf);
free(buf);
}
Modified: nnrpd/nnrpd.c
===================================================================
--- nnrpd/nnrpd.c 2016-09-04 12:48:39 UTC (rev 10061)
+++ nnrpd/nnrpd.c 2016-09-04 12:52:31 UTC (rev 10062)
@@ -790,6 +790,10 @@
ssize_t len;
len = vsnprintf(buff, sizeof(buff), fmt, args);
+ if (len < 0)
+ len = 0;
+ else if ((size_t)len >= sizeof(buff))
+ len = sizeof(buff) - 1;
write_buffer(buff, len);
if (dotrace && Tracing) {
More information about the inn-committers
mailing list