id,summary,reporter,owner,description,type,status,priority,milestone,component,version,severity,resolution,keywords,cc 894,(v)s(n)printf hardening,gvranken,,"vsnprintf is a function that can fail. It will always fail for strings that require over 2 gigabytes of buffer space, for the simple reason that this size cannot unambiguously be expressed in its return value; a return value of >= 0 means that that many bytes were written to the buffer, and a negative return value indicates overall failure. Because the largest positive value of a signed int is 2 gigabytes, buffer consumption is limited to that amount. For some format strings, snprintf will also internally allocate (usually small) heap buffers, and will return failure if any of these allocations fail. I believe the FreeBSD internal dtoa() function allocates heap memory, for instance. A memory leak in the main program that can grow so large that vsnprintf cannot allocate its private buffers, will result in failure. In principle, the main program (OpenVPN) can never assume that a call to vsnprintf succeeds, unless it returns a non-negative integer. This is the current openvpn_snprintf in openvpn: {{{ 277 bool 278 openvpn_snprintf(char *str, size_t size, const char *format, ...) 279 { 280 va_list arglist; 281 int len = -1; 282 if (size > 0) 283 { 284 va_start(arglist, format); 285 len = vsnprintf(str, size, format, arglist); 286 va_end(arglist); 287 str[size - 1] = 0; 288 } 289 return (len >= 0 && len < size); 290 } }}} The vsnprintf specifications do not give a clear definition on what the contents of the output buffer may be if the function fails. Hence, if the call to vsnprintf within openvpn_snprintf fails, the space str[0..size-1] may be uninitialized data. Many calls to openvpn_snprintf do not check its return value, so if vsnprintf fails, and the buffer contents is subsequently written to the peer (or to any other channel, like a file), this could constitute a significant data leak. Also overlapping parameters will produce undefined buffer contents. man vsnprintf explicitly says: {{{ Some programs imprudently rely on code such as the following sprintf(buf, ""%s some further text"", buf); to append text to buf. However, the standards explicitly note that the results are undefined if source and destination buffers overlap when calling sprintf(), snprintf(), vsprintf(), and vsnprintf(). Depending on the version of gcc(1) used, and the compiler options employed, calls such as the above will not produce the expected results. }}} These are definitely corner cases, and I'm not aware of an easy to to force vsnprintf failure in openvpn. But the stakes are high (private crypto data and what have you), and other bugs (excessive string generation that is then sprintf'ed, overlapping strings, memory shortage) might lead to a significant security problem this way, so my I'm attaching a patch that ensures that the final contents of the output string is always a valid string that does not contain uninitialized or unrelated data (barring any bugs in the implementation of vsnprintf, which is outside OpenVPN's responsibility or control). Guido",Patch submission,new,major,,Generic / unclassified,OpenVPN 2.2.2 (Community Ed),"Not set (select this one, unless your'e a OpenVPN developer)",,,