> On 06.05.2016, at 17:56, Hiltjo Posthuma <[email protected]> wrote: > > On Wed, Apr 27, 2016 at 2:41 PM, Hiltjo Posthuma <[email protected] > <mailto:[email protected]>> wrote: >> Hi, >> >> The following patch for httpd makes sure the value of the asprintf buffer is >> zeroed on error and not relied upon, so at the 'done' label free(body) and >> free(hstsheader) is safe. >> >> from asprintf(3): >> >> "The asprintf() and vasprintf() functions return the number of >> characters >> that were output to the newly allocated string (excluding the '\0'). >> A pointer to the newly allocated string is returned in ret; it >> should be passed to free(3) to release the allocated storage when it >> is >> no longer needed. If sufficient space cannot be allocated, these >> functions will return -1. >>The value of ret in this situation is >> implementation-dependent (on OpenBSD, ret will be set to the null >> pointer, but this behavior should not be relied upon)."<< >> >> >> Index: server_http.c >> =================================================================== >> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v >> retrieving revision 1.106 >> diff -u -p -r1.106 server_http.c >> --- server_http.c 8 Mar 2016 09:33:15 -0000 1.106 >> +++ server_http.c 27 Apr 2016 12:01:00 -0000 >> @@ -826,8 +826,10 @@ server_abort_http(struct client *clt, un >> "<hr>\n<address>%s</address>\n" >> "</body>\n" >> "</html>\n", >> - code, httperr, style, code, httperr, HTTPD_SERVERNAME)) == -1) >> + code, httperr, style, code, httperr, HTTPD_SERVERNAME)) == -1) { >> + body = NULL; >> goto done; >> + } >> >> if (srv_conf->flags & SRVFLAG_SERVER_HSTS) { >> if (asprintf(&hstsheader, "Strict-Transport-Security: " >> @@ -835,8 +837,10 @@ server_abort_http(struct client *clt, un >> srv_conf->hsts_flags & HSTSFLAG_SUBDOMAINS ? >> "; includeSubDomains" : "", >> srv_conf->hsts_flags & HSTSFLAG_PRELOAD ? >> - "; preload" : "") == -1) >> + "; preload" : "") == -1) { >> + hstsheader = NULL; >> goto done; >> + } >> } >> >> /* Add basic HTTP headers */ >> >> -- > > Any OK's for this? > > Please also see my mail with subject "httpd: fix/style: unbalanced > va_start and va_end macros" (don't want to spam the mailinglist :)).
I don't know. It doesn't affect OpenBSD and I don't like portability code in tree. Isn't it better to create a portable version of httpd instead? If OpenBSD's behavior of asprintf is non-standard and everyone else is doing it differently, we would probably have to apply the patch. But this would also affect many other places in the tree were we rely on our asprintf semantics. Reyk
