@cryptotaxi247 / netdata-1 / commits / 3bf19c3b2

resolved buffer overflow in snprintfz(); code cleanup;

Costa Tsaousis committed May 13, 2016 at 21:08 UTC 3bf19c3b2f8618dfda5bb63b41ce22b6a055b4a8
5 files changed +45 -30
src/common.c
+21 -15
@@ -796,31 +796,37 @@ char *fgets_trim_len(char *buf, size_t buf_size, FILE *fp, size_t *len) {
796 return s;
797 }
798
799 -char *strncpyz(char *dest, const char *src, size_t n)
800 -{
801 - char *p = dest;
799 +char *strncpyz(char *dst, const char *src, size_t n) {
800 + char *p = dst;
801 +
802 + while(*src && n--)
803 + *dst++ = *src++;
804
803 - while (*src && n--)
804 - *dest++ = *src++;
805 - *dest = '\0';
805 + *dst = '\0';
806
807 return p;
808 }
809
810 -int vsnprintfz(char *sout, size_t n, const char *fmt, va_list args)
811 -{
810 +int vsnprintfz(char *dst, size_t n, const char *fmt, va_list args) {
811 int size;
812
814 - size = vsnprintf(sout, n, fmt, args);
815 - sout[size] = '\0';
816 - return size;
813 + size = vsnprintf(dst, n, fmt, args);
814 +
815 + if(unlikely((size_t)size > n)) {
816 + // there is bug in vsnprintf() and it returns
817 + // a number higher to len, but it does not
818 + // overflow the buffer.
819 + size = n;
820 + }
821 +
822 + dst[size] = '\0';
823 + return size;
824 }
825
819 -int snprintfz(char *sout, size_t n, const char *fmt, ...)
820 -{
826 +int snprintfz(char *dst, size_t n, const char *fmt, ...) {
827 va_list args;
828
829 va_start(args, fmt);
824 - return vsnprintfz(sout, n, fmt, args);
825 - // args will vanish!
830 + return vsnprintfz(dst, n, fmt, args);
831 + va_end(args);
832 }
src/common.h
+4 -3
@@ -24,9 +24,10 @@ extern uint32_t simple_hash(const char *name);
24 extern void strreverse(char* begin, char* end);
25 extern char *mystrsep(char **ptr, char *s);
26 extern char *trim(char *s);
27 -extern char *strncpyz(char *dest, const char *src, size_t n);
28 -extern int vsnprintfz(char *sout, size_t n, const char *fmt, va_list args);
29 -extern int snprintfz(char *sout, size_t n, const char *fmt, ...);
27 +
28 +extern char *strncpyz(char *dst, const char *src, size_t n);
29 +extern int vsnprintfz(char *dst, size_t n, const char *fmt, va_list args);
30 +extern int snprintfz(char *dst, size_t n, const char *fmt, ...);
31
32 extern void *mymmap(const char *filename, size_t size, int flags, int ksm);
33 extern int savememory(const char *filename, void *mem, unsigned long size);
src/web_buffer.c
+11 -9
@@ -80,13 +80,14 @@ void buffer_strcat(BUFFER *wb, const char *txt)
80
81 buffer_need_bytes(wb, 1);
82
83 - char *s = &wb->buffer[wb->len], *end = &wb->buffer[wb->size];
83 + char *s = &wb->buffer[wb->len], *start, *end = &wb->buffer[wb->size];
84 long len = wb->len;
85
86 - while(*txt && s != end) {
86 + start = s;
87 + while(*txt && s != end)
88 *s++ = *txt++;
88 - len++;
89 - }
89 +
90 + len += s - start;
91
92 wb->len = len;
93 buffer_overflow_check(wb);
@@ -110,7 +111,7 @@ void buffer_snprintf(BUFFER *wb, size_t len, const char *fmt, ...)
111 {
112 if(unlikely(!fmt || !*fmt)) return;
113
113 - buffer_need_bytes(wb, len+1);
114 + buffer_need_bytes(wb, len + 1);
115
116 va_list args;
117 va_start(args, fmt);
@@ -126,9 +127,9 @@ void buffer_vsprintf(BUFFER *wb, const char *fmt, va_list args)
127 {
128 if(unlikely(!fmt || !*fmt)) return;
129
129 - buffer_need_bytes(wb, 1);
130 + buffer_need_bytes(wb, 2);
131
131 - size_t len = wb->size - wb->len;
132 + size_t len = wb->size - wb->len - 1;
133
134 wb->len += vsnprintfz(&wb->buffer[wb->len], len, fmt, args);
135
@@ -141,9 +142,10 @@ void buffer_sprintf(BUFFER *wb, const char *fmt, ...)
142 {
143 if(unlikely(!fmt || !*fmt)) return;
144
144 - buffer_need_bytes(wb, 1);
145 + buffer_need_bytes(wb, 2);
146
146 - size_t len = wb->size - wb->len, wrote;
147 + size_t len = wb->size - wb->len - 1;
148 + size_t wrote;
149
150 va_list args;
151 va_start(args, fmt);
src/web_buffer.h
+1 -1
@@ -48,7 +48,7 @@ extern const char *buffer_tostring(BUFFER *wb);
48
49 #define buffer_need_bytes(buffer, needed_free_size) do { if(unlikely((buffer)->size - (buffer)->len < (size_t)(needed_free_size))) buffer_increase((buffer), (size_t)(needed_free_size)); } while(0)
50
51 -#define buffer_flush(wb) wb->buffer[wb->len = 0] = '\0'
51 +#define buffer_flush(wb) wb->buffer[(wb)->len = 0] = '\0'
52 extern void buffer_reset(BUFFER *wb);
53
54 extern void buffer_strcat(BUFFER *wb, const char *txt);
src/web_server.c
+8 -2
@@ -79,7 +79,10 @@ int create_listen_socket4(const char *ip, int port, int listen_backlog)
79 /* avoid "address already in use" */
80 setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*)&sockopt, sizeof(sockopt));
81
82 - struct sockaddr_in name = { .sin_family = AF_INET, .sin_port = htons(port) };
82 + struct sockaddr_in name;
83 + memset(&name, 0, sizeof(struct sockaddr_in));
84 + name.sin_family = AF_INET;
85 + name.sin_port = htons (port);
86
87 if(is_ip_anything(ip)) {
88 name.sin_addr.s_addr = htonl(INADDR_ANY);
@@ -127,7 +130,10 @@ int create_listen_socket6(const char *ip, int port, int listen_backlog)
130 /* avoid "address already in use" */
131 setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void*)&sockopt, sizeof(sockopt));
132
130 - struct sockaddr_in6 name = { .sin6_family = AF_INET6, .sin6_port = htons((uint16_t)port) };
133 + struct sockaddr_in6 name;
134 + memset(&name, 0, sizeof(struct sockaddr_in6));
135 + name.sin6_family = AF_INET6;
136 + name.sin6_port = htons ((uint16_t) port);
137
138 if(is_ip_anything(ip)) {
139 name.sin6_addr = in6addr_any;