From a875d3fb4beda43cb54b5810565bafc16a568e5c Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Sun, 12 Mar 2017 18:27:45 +0100 Subject: Fix possible truncated output in ipset output buffer handling Omri Bahumi and Yoni Lavi discovered that due to the inproper handling of the ipset output buffer, the output may be truncated. So for example in an "ipset save" output, instead of 192.168.0.0/24, just 192.168.0.0 printed. If one use "ipset save" and then "ipset restore" to restore the sets, this may lead to wrong firewall rules at the end. The patch fixes the bug in the ipset code. --- lib/print.c | 2 +- lib/session.c | 73 +++++++++++++++++++++++++++++++---------------------------- 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/lib/print.c b/lib/print.c index 7f42434..7dd229e 100644 --- a/lib/print.c +++ b/lib/print.c @@ -31,7 +31,7 @@ #define SNPRINTF_FAILURE(size, len, offset) \ do { \ if (size < 0 || (unsigned int) size >= len) \ - return size; \ + return offset + size; \ offset += size; \ len -= size; \ } while (0) diff --git a/lib/session.c b/lib/session.c index 24f29f5..1bdaaa7 100644 --- a/lib/session.c +++ b/lib/session.c @@ -706,33 +706,47 @@ call_outfn(struct ipset_session *session) /* Handle printing failures */ static jmp_buf printf_failure; -static int __attribute__((format(printf, 2, 3))) -safe_snprintf(struct ipset_session *session, const char *fmt, ...) +static int +handle_snprintf_error(struct ipset_session *session, + int len, int ret, int loop) { - va_list args; - int len, ret, loop = 0; - -retry: - len = strlen(session->outbuf); - D("len: %u, retry %u", len, loop); - va_start(args, fmt); - ret = vsnprintf(session->outbuf + len, IPSET_OUTBUFLEN - len, - fmt, args); - va_end(args); - if (ret < 0 || ret >= IPSET_OUTBUFLEN - len) { /* Buffer was too small, push it out and retry */ - D("print buffer and try again: %u", len); - if (loop++) { + D("print buffer and try again: len: %u, ret: %d", len, ret); + if (loop) { ipset_err(session, "Internal error at printing, loop detected!"); longjmp(printf_failure, 1); } session->outbuf[len] = '\0'; - if (!call_outfn(session)) - goto retry; + if (call_outfn(session)) { + ipset_err(session, + "Internal error, could not print output buffer!"); + longjmp(printf_failure, 1); + } + return 1; } + return 0; +} + +static int __attribute__((format(printf, 2, 3))) +safe_snprintf(struct ipset_session *session, const char *fmt, ...) +{ + va_list args; + int len, ret, loop = 0; + + do { + len = strlen(session->outbuf); + D("len: %u, retry %u", len, loop); + va_start(args, fmt); + ret = vsnprintf(session->outbuf + len, + IPSET_OUTBUFLEN - len, + fmt, args); + va_end(args); + loop = handle_snprintf_error(session, len, ret, loop); + } while (loop); + return ret; } @@ -742,25 +756,14 @@ safe_dprintf(struct ipset_session *session, ipset_printfn fn, { int len, ret, loop = 0; -retry: - len = strlen(session->outbuf); - D("len: %u, retry %u", len, loop); - ret = fn(session->outbuf + len, IPSET_OUTBUFLEN - len, - session->data, opt, session->envopts); - - if (ret < 0 || ret >= IPSET_OUTBUFLEN - len) { - /* Buffer was too small, push it out and retry */ - D("print buffer and try again: %u", len); - if (loop++) { - ipset_err(session, - "Internal error at printing, loop detected!"); - longjmp(printf_failure, 1); - } + do { + len = strlen(session->outbuf); + D("len: %u, retry %u", len, loop); + ret = fn(session->outbuf + len, IPSET_OUTBUFLEN - len, + session->data, opt, session->envopts); + loop = handle_snprintf_error(session, len, ret, loop); + } while (loop); - session->outbuf[len] = '\0'; - if (!call_outfn(session)) - goto retry; - } return ret; } -- cgit v1.2.3