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.

diff --git a/lib/print.c b/lib/print.c
index 7f42434f78e4fdef025e7ee3e7d9a5e9e820326f..7dd229ef8718adfc0bd43d6fc1ced5c2e6b412b2 100644 (file)
--- 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 24f29f592ed8c6af41f084807bbd61a6246d6939..1bdaaa7045f6165341d384bfd66dcd8afdda3fef 100644 (file)
--- 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;
 }