From f043a400e25bd6a8bd0205aebebcfa398576b1a3 Mon Sep 17 00:00:00 2001 From: ahgamut <41098605+ahgamut@users.noreply.github.com> Date: Sat, 10 Jul 2021 10:24:34 +0530 Subject: [PATCH] changes made after review --- libc/dns/getaddrinfo.c | 6 +++--- libc/dns/getservbyname.c | 6 ++++-- libc/dns/getservbyport.c | 6 ++++-- libc/dns/prototxt.c | 19 +++++++++++------- libc/dns/servicestxt.c | 33 +++++++++++++++++++++---------- test/libc/dns/prototxt_test.c | 10 ++++++++++ test/libc/dns/servicestxt_test.c | 34 ++++++++++++++++++++++++++++---- 7 files changed, 86 insertions(+), 28 deletions(-) diff --git a/libc/dns/getaddrinfo.c b/libc/dns/getaddrinfo.c index 103f8f764..015cc61d5 100644 --- a/libc/dns/getaddrinfo.c +++ b/libc/dns/getaddrinfo.c @@ -55,11 +55,11 @@ int getaddrinfo(const char *name, const char *service, if (!name && (hints->ai_flags & AI_CANONNAME)) return EAI_BADFLAGS; if (service) { if (hints->ai_socktype == SOCK_STREAM) - strncpy(proto, "tcp", sizeof(proto)); + strcpy(proto, "tcp"); else if (hints->ai_socktype == SOCK_DGRAM) - strncpy(proto, "udp", sizeof(proto)); + strcpy(proto, "udp"); else /* ai_socktype == 0 */ - proto[0] = '\0'; + strcpy(proto, ""); if ((port = LookupServicesByName(service, proto, sizeof(proto), NULL, 0, NULL)) == -1) { diff --git a/libc/dns/getservbyname.c b/libc/dns/getservbyname.c index 62fcf0168..4c20cc43a 100644 --- a/libc/dns/getservbyname.c +++ b/libc/dns/getservbyname.c @@ -44,8 +44,10 @@ struct servent *getservbyname(const char *name, const char *proto) { ptr0 = &se0; } - localproto[0] = '\0'; - if (proto) strncpy(localproto, proto, DNS_NAME_MAX); + if (proto) { + if (!memccpy(localproto, proto, '\0', DNS_NAME_MAX)) return NULL; + } else + strcpy(localproto, ""); p = LookupServicesByName(name, ptr0->s_proto, DNS_NAME_MAX, ptr0->s_name, DNS_NAME_MAX, NULL); diff --git a/libc/dns/getservbyport.c b/libc/dns/getservbyport.c index fc4e21c49..891766b7c 100644 --- a/libc/dns/getservbyport.c +++ b/libc/dns/getservbyport.c @@ -43,8 +43,10 @@ struct servent *getservbyport(int port, const char *proto) { ptr1 = &se1; } - localproto[0] = '\0'; - if (proto) strncpy(localproto, proto, DNS_NAME_MAX); + if (proto) { + if (!memccpy(localproto, proto, '\0', DNS_NAME_MAX)) return NULL; + } else + strcpy(localproto, ""); if (LookupServicesByPort(ntohs(port), ptr1->s_proto, DNS_NAME_MAX, ptr1->s_name, DNS_NAME_MAX, NULL) == -1) diff --git a/libc/dns/prototxt.c b/libc/dns/prototxt.c index 816f6325e..5195deeed 100644 --- a/libc/dns/prototxt.c +++ b/libc/dns/prototxt.c @@ -57,11 +57,10 @@ static textwindows noinline char *GetNtProtocolsTxtPath(char *pathbuf, * * format of /etc/protocols is like this: * - * # comment - * # NAME PROTOCOL ALIASES - * - * ip 0 IP - * icmp 1 ICMP + * # comment + * # NAME PROTOCOL ALIASES + * ip 0 IP + * icmp 1 ICMP * * @param protonum is the protocol number * @param buf is a buffer to store the official name of the protocol @@ -102,7 +101,10 @@ int LookupProtoByNumber(const int protonum, char *buf, size_t bufsize, name = strtok_r(line, " \t\r\n\v", &tok); number = strtok_r(NULL, " \t\r\n\v", &tok); if (name && number && protonum == atoi(number)) { - strncpy(buf, name, bufsize); + if (!memccpy(buf, name, '\0', bufsize)) { + strcpy(buf, ""); + break; + } found = 1; } } @@ -170,8 +172,11 @@ int LookupProtoByName(const char *protoname, char *buf, size_t bufsize, if (alias) /* alias matched with protoname */ { + if (!memccpy(buf, name, '\0', bufsize)) { + strcpy(buf, ""); + break; + } result = atoi(number); - strncpy(buf, name, bufsize); found = 1; } } diff --git a/libc/dns/servicestxt.c b/libc/dns/servicestxt.c index cb45535d0..8784fa11d 100644 --- a/libc/dns/servicestxt.c +++ b/libc/dns/servicestxt.c @@ -56,12 +56,11 @@ static textwindows noinline char *GetNtServicesTxtPath(char *pathbuf, * * format of /etc/services is like this: * - * # comment - * # NAME PORT/PROTOCOL ALIASES - * - * ftp 21/tcp - * fsp 21/udp fspd - * ssh 22/tcp + * # comment + * # NAME PORT/PROTOCOL ALIASES + * ftp 21/tcp + * fsp 21/udp fspd + * ssh 22/tcp * * @param servport is the port number * @param servproto is a NULL-terminated string (eg "tcp", "udp") @@ -102,6 +101,7 @@ int LookupServicesByPort(const int servport, char *servproto, line = NULL; linesize = 0; found = 0; + strcpy(buf, ""); while (found == 0 && (getline(&line, &linesize, f)) != -1) { if ((comment = strchr(line, '#'))) *comment = '\0'; @@ -110,9 +110,15 @@ int LookupServicesByPort(const int servport, char *servproto, proto = strtok_r(NULL, " \t\r\n\v", &tok); if (name && port && proto && servport == atoi(port)) { if (!servproto[0] || strncasecmp(proto, servproto, servprotolen) == 0) { - strncpy(buf, name, bufsize); + if (!servproto[0] && !memccpy(servproto, proto, '\0', servprotolen)) { + strcpy(servproto, ""); + break; + } + if (!memccpy(buf, name, '\0', bufsize)) { + strcpy(buf, ""); + break; + } found = 1; - if (!servproto[0]) strncpy(servproto, proto, servprotolen); } } } @@ -175,6 +181,7 @@ int LookupServicesByName(const char *servname, char *servproto, linesize = 0; found = 0; result = -1; + if (buf && bufsize != 0) strcpy(buf, ""); while (found == 0 && (getline(&line, &linesize, f)) != -1) { if ((comment = strchr(line, '#'))) *comment = '\0'; @@ -189,10 +196,16 @@ int LookupServicesByName(const char *servname, char *servproto, if (alias) /* alias matched with servname */ { if (!servproto[0] || strncasecmp(proto, servproto, servprotolen) == 0) { + if (!servproto[0] && !memccpy(servproto, proto, '\0', servprotolen)) { + strcpy(servproto, ""); + break; + } + if (buf && bufsize != 0 && !memccpy(buf, name, '\0', bufsize)) { + strcpy(buf, ""); + break; + } result = atoi(port); found = 1; - if (!servproto[0]) strncpy(servproto, proto, servprotolen); - if (buf && bufsize != 0) strncpy(buf, name, bufsize); } } } diff --git a/test/libc/dns/prototxt_test.c b/test/libc/dns/prototxt_test.c index 548460933..d3c457926 100644 --- a/test/libc/dns/prototxt_test.c +++ b/test/libc/dns/prototxt_test.c @@ -48,10 +48,15 @@ ggp 3 GGP "; TEST(LookupProtoByNumber, GetNameWhenNumberCorrect) { char name[16]; /* sample has only names of length 3-4 */ + strcpy(name, ""); ASSERT_EQ(-1, /*non-existent number */ LookupProtoByNumber(24, name, sizeof(name), "protocols")); + ASSERT_EQ(-1, /* sizeof(name) insufficient, memccpy failure */ + LookupProtoByNumber(73, name, 1, "protocols")); + ASSERT_STREQ(name, ""); /* cleaned up after memccpy failed */ + ASSERT_EQ(0, /* works with valid number */ LookupProtoByNumber(73, name, sizeof(name), "protocols")); ASSERT_STREQ(name, "rspf"); /* official name written */ @@ -59,10 +64,15 @@ TEST(LookupProtoByNumber, GetNameWhenNumberCorrect) { TEST(LookupProtoByName, GetNumberWhenNameOrAlias) { char name[16]; /* sample has only names of length 3-4 */ + strcpy(name, ""); ASSERT_EQ(-1, /* non-existent name or alias */ LookupProtoByName("tcp", name, sizeof(name), "protocols")); + ASSERT_EQ(-1, /* sizeof(name) insufficient, memccpy failure */ + LookupProtoByName("ggp", name, 1, "protocols")); + ASSERT_STREQ(name, ""); /* cleaned up after memccpy failed */ + ASSERT_EQ(3, /* works with valid name */ LookupProtoByName("ggp", name, sizeof(name), "protocols")); ASSERT_STREQ(name, "ggp"); diff --git a/test/libc/dns/servicestxt_test.c b/test/libc/dns/servicestxt_test.c index 1f5b50bc8..0be5d810e 100644 --- a/test/libc/dns/servicestxt_test.c +++ b/test/libc/dns/servicestxt_test.c @@ -53,8 +53,8 @@ TEST(LookupServicesByPort, GetNameWhenPortCorrect) { char proto1[] = "tcp"; char proto2[] = "udp"; char* localproto; - - eitherproto[0] = '\0'; + strcpy(eitherproto, ""); + strcpy(name, ""); localproto = eitherproto; ASSERT_EQ(-1, /* non existent port */ @@ -80,6 +80,19 @@ TEST(LookupServicesByPort, GetNameWhenPortCorrect) { LookupServicesByPort(22, localproto, 0, name, sizeof(name), "services")); ASSERT_STREQ(proto1, "tcp"); + localproto = proto1; + ASSERT_EQ(-1, /* sizeof(name) insufficient, memccpy failure */ + LookupServicesByPort(22, localproto, sizeof(proto1), name, 1, + "services")); + ASSERT_STREQ(proto1, "tcp"); + ASSERT_STREQ(name, ""); /* cleaned up after memccpy failed */ + + localproto = eitherproto; + ASSERT_EQ( + -1, /* sizeof(proto) insufficient, memccpy failure */ + LookupServicesByPort(22, localproto, 1, name, sizeof(name), "services")); + ASSERT_STREQ(eitherproto, ""); /* cleaned up after memccpy failed */ + localproto = proto1; ASSERT_EQ(0, LookupServicesByPort(22, localproto, sizeof(proto1), name, sizeof(name), "services")); @@ -107,7 +120,8 @@ TEST(LookupServicesByName, GetPortWhenNameOrAlias) { char proto1[] = "tcp"; char proto2[] = "udp"; char* localproto; - eitherproto[0] = '\0'; + strcpy(eitherproto, ""); + strcpy(name, ""); localproto = eitherproto; ASSERT_EQ(-1, /* non-existent name */ @@ -127,6 +141,19 @@ TEST(LookupServicesByName, GetPortWhenNameOrAlias) { "services")); ASSERT_STREQ(proto2, "udp"); + localproto = proto1; + ASSERT_EQ(-1, /* sizeof(name) insufficient, memccpy failure */ + LookupServicesByName("ssh", localproto, sizeof(proto1), name, 1, + "services")); + ASSERT_STREQ(proto1, "tcp"); + ASSERT_STREQ(name, ""); /* cleaned up after memccpy failed */ + + localproto = eitherproto; + ASSERT_EQ(-1, /* sizeof(proto) insufficient, memccpy failure */ + LookupServicesByName("ssh", localproto, 1, name, sizeof(name), + "services")); + ASSERT_STREQ(eitherproto, ""); /* cleaned up after memccpy failed */ + localproto = proto1; ASSERT_EQ(22, LookupServicesByName("ssh", localproto, sizeof(proto1), name, sizeof(name), "services")); @@ -144,7 +171,6 @@ TEST(LookupServicesByName, GetPortWhenNameOrAlias) { LookupServicesByName("ttytst", localproto, sizeof(proto2), NULL, 0, "services")); - name[0] = '\0'; localproto = eitherproto; ASSERT_EQ(19, /* pick first matching protocol */ LookupServicesByName("source", localproto, sizeof(eitherproto),