Small demonstration of the kinds of things we'll have to mop up for
weeks more.
>From OpenSSL CHANGES file:
*) Introduce safe string copy and catenation functions
(BUF_strlcpy() and BUF_strlcat()).
[Ben Laurie (CHATS) and Richard Levitte]
That's from back in 2002.
These functions work just like ours in OpenBSD. The return values
indicate overflow. We've been advising people to check for overflow
from the start, which is why we designed these like snprintf.
Good move, but then let's see how they used them:
./apps/ca.c: BUF_strlcpy(tofree, s, len);
./apps/ca.c: BUF_strlcat(tofree, "/", len);
./apps/ca.c: BUF_strlcat(tofree, CONFIG_FILE, len);
./apps/ca.c: BUF_strlcat(buf[2], "/", sizeof(buf[2]));
./apps/ca.c: BUF_strlcpy(row[DB_file], "unknown", 8);
./apps/ca.c: BUF_strlcpy(row[DB_file], "unknown", 8);
./apps/ca.c: BUF_strlcpy(str, (char *) revtm->data, i);
./apps/ca.c: BUF_strlcat(str, ",", i);
./apps/ca.c: BUF_strlcat(str, reason, i);
./apps/ca.c: BUF_strlcat(str, ",", i);
./apps/ca.c: BUF_strlcat(str, other, i);
./apps/apps.c: BUF_strlcpy(out, p, size);
./apps/apps.c: BUF_strlcpy(buf[0], serialfile, BSIZE);
./apps/engine.c: BUF_strlcat(*buf, ", ", *size);
./apps/engine.c: BUF_strlcat(*buf, s, *size);
./apps/pkcs12.c: BUF_strlcpy(macpass, pass, sizeof
macpass);
./apps/pkcs12.c: BUF_strlcpy(macpass, pass, sizeof macpass);
./apps/req.c: BUF_strlcpy(buf, value, sizeof buf);
./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf);
./apps/req.c: BUF_strlcpy(buf, def, sizeof buf);
./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf);
./apps/req.c: BUF_strlcpy(buf, value, sizeof buf);
./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf);
./apps/req.c: BUF_strlcpy(buf, def, sizeof buf);
./apps/req.c: BUF_strlcat(buf, "\n", sizeof buf);
./apps/s_socket.c: BUF_strlcpy(*host, h1->h_name,
strlen(h1->h_name) + 1);
./apps/srp.c: BUF_strlcpy(tofree, s, len);
./apps/srp.c: BUF_strlcat(tofree, "/", len);
./apps/srp.c: BUF_strlcat(tofree, CONFIG_FILE, len);
./apps/x509.c: BUF_strlcpy(buf, CAfile, len);
./apps/x509.c: BUF_strlcat(buf, POSTFIX, len);
./apps/x509.c: BUF_strlcpy(buf, serialfile, len);
./crypto/asn1/a_time.c: if (t->data[0] >= '5') BUF_strlcpy(str, "19", newlen);
./crypto/asn1/a_time.c: else BUF_strlcpy(str, "20", newlen);
./crypto/asn1/a_time.c: BUF_strlcat(str, (char *)t->data, newlen);
./crypto/bio/b_dump.c: BUF_strlcpy(buf, str, sizeof buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, " ", sizeof
buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof
buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, " ", sizeof buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, tmp, sizeof buf);
./crypto/bio/b_dump.c: BUF_strlcat(buf, "\n", sizeof buf);
./crypto/bio/bss_file.c: BUF_strlcpy(p, "a+",
sizeof p);
./crypto/bio/bss_file.c: else BUF_strlcpy(p, "a",
sizeof p);
./crypto/bio/bss_file.c: BUF_strlcpy(p, "r+", sizeof p);
./crypto/bio/bss_file.c: BUF_strlcpy(p, "w", sizeof p);
./crypto/bio/bss_file.c: BUF_strlcpy(p, "r", sizeof p);
./crypto/buffer/buf_str.c:BUF_strlcpy(char *dst, const char *src, size_t size)
./crypto/buffer/buf_str.c:BUF_strlcat(char *dst, const char *src, size_t size)
./crypto/buffer/buffer.h:size_t BUF_strlcpy(char *dst, const char *src, size_t
siz);
./crypto/buffer/buffer.h:size_t BUF_strlcat(char *dst, const char *src, size_t
siz);
./crypto/conf/conf_def.c: BUF_strlcpy(section,"default",10);
./crypto/conf/conf_def.c:
BUF_strlcpy(v->name,pname,strlen(pname)+1);
./crypto/dso/dso_lib.c: BUF_strlcpy(copied, filename, strlen(filename) + 1);
./crypto/dso/dso_lib.c: BUF_strlcpy(result, filename, strlen(filename)
+ 1);
./crypto/err/err.c: BUF_strlcat(str,a,(size_t)s+1);
./crypto/evp/evp_pbe.c: if (!pbe_obj) BUF_strlcpy (obj_tmp, "NULL",
sizeof obj_tmp);
./crypto/objects/obj_dat.c:
BUF_strlcpy(buf,s,buf_len);
./crypto/objects/obj_dat.c:
BUF_strlcpy(buf,bndec,buf_len);
./crypto/objects/obj_dat.c:
BUF_strlcpy(buf,tbuf,buf_len);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,"Proc-Type: 4,",PEM_BUFSIZE);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,str,PEM_BUFSIZE);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,"\n",PEM_BUFSIZE);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,"DEK-Info: ",PEM_BUFSIZE);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,type,PEM_BUFSIZE);
./crypto/pem/pem_lib.c: BUF_strlcat(buf,",",PEM_BUFSIZE);
./crypto/rand/randfile.c: if (BUF_strlcpy(buf,"/dev/urandom",size) >=
size)
./crypto/ui/ui_lib.c: BUF_strlcpy(prompt, prompt1, len + 1);
./crypto/ui/ui_lib.c: BUF_strlcat(prompt, object_desc, len + 1);
./crypto/ui/ui_lib.c: BUF_strlcat(prompt, prompt2, len + 1);
./crypto/ui/ui_lib.c: BUF_strlcat(prompt, object_name, len +
1);
./crypto/ui/ui_lib.c: BUF_strlcat(prompt, prompt3, len + 1);
./crypto/ui/ui_lib.c: BUF_strlcpy(uis->result_buf, result,
./crypto/x509v3/v3_info.c: BUF_strlcpy(ntmp, objtmp, nlen);
./crypto/x509v3/v3_info.c: BUF_strlcat(ntmp, " - ", nlen);
./crypto/x509v3/v3_info.c: BUF_strlcat(ntmp, vtmp->name, nlen);
Lots of overflow checks missing. It is possible that some of those
are range-checked beforehands, but I'd put the odds as low.
About 100 snprintf's are handled the same way...
The OpenSSL developers are not responsible programmers. They are
uneducated regarding the concept of 'best practice'.
