Send inn-workers mailing list submissions to
[email protected]
To subscribe or unsubscribe via the World Wide Web, visit
https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."
Today's Topics:
1. Re: [PATCH] fix snprintf return value misuse (and some
related off-by-1/etc) (Julien ?LIE)
----------------------------------------------------------------------
Message: 1
Date: Thu, 1 Sep 2016 21:31:14 +0200
From: Julien ?LIE <[email protected]>
To: [email protected]
Cc: "Yuriy M. Kaminskiy" <[email protected]>
Subject: Re: [PATCH] fix snprintf return value misuse (and some
related off-by-1/etc)
Message-ID: <[email protected]>
Content-Type: text/plain; charset=windows-1252
Hi Yuriy,
> snprintf() return -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.
> As I don't use inn, patch is compile-tested only, passes `make check`.
Thanks a lot for your patches, that I did not forget and marked
to be integrated to the next release. I've at last taken the time
to do that work, and they will show up in INN 2.6.1.
--- lib/timer.c (revision 9984)
+++ lib/timer.c (working copy)
if (off == len) {
warn("timer log too long while processing %s",
TMRlabel(labels, timer->id));
- return 0;
+ return off;
}
You even find a genuine bug, fixed with the above patch you submitted.
Thanks for that catch!
--- lib/timer.c (revision 9984)
+++ lib/timer.c (working copy)
@@ -369,14 +387,36 @@
- off += snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
+ rc = snprintf(buf + off, len - off, "time %lu ", TMRgettime(true));
+ if (rc < 0) {
+ /* do nothing */
+ } else if ((size_t)rc >= len) {
+ off = len;
+ } else {
+ off += rc;
+ }
Hmm, are you sure the check is "if ((size_t)rc >= len)"
and not "if ((size_t)rc >= len - off)"?
I think "len - off" is really needed because otherwise the returned
value could be larger than the supplied buffer size of "len - off".
--- lib/timer.c (revision 9984)
+++ lib/timer.c (working copy)
@@ -359,6 +376,7 @@
for (i = 0; i < timer_count; i++)
- if (timers[i] != NULL)
- off += TMRsumone(labels, timers[i], buf + off, len - off);
+ if (timers[i] != NULL) {
+ rc = TMRsumone(labels, timers[i], buf + off, len - off);
+ if (rc < 0) {
+ /* do nothing */
+ } else if ((size_t)rc >= len) {
+ off = len;
+ } else {
+ off += rc;
+ }
+ }
Is that change really necessary, as TMRsumone() can no longer return
a negative value or a value greater than the supplied buffer size?
I would just not change anything there.
Thanks again for your work,
--
Julien ?LIE
? Petite annonce : Abeille ?pouserait frelon. Lune de miel
assur?e. ?
------------------------------
Subject: Digest Footer
_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers
------------------------------
End of inn-workers Digest, Vol 87, Issue 1
******************************************
