← Commit list

7895b9174

Denys Vlasenko
Denys Vlasenko
2009-07-03
FAILED Duration: 33s

Subject

udhcp: dname_dec may return NULL, account for that case

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

networking/udhcp/clientpacket.c +2 -2networking/udhcp/domain_codec.c +39 -34networking/udhcp/options.c +1 -1networking/udhcp/options.h +5 -3networking/udhcp/script.c +19 -18
networking/udhcp/clientpacket.c+2 -2
@@ -166,7 +166,7 @@ int FAST_FUNC send_select(uint32_t xid, uint32_t server, uint32_t requested)
}
-/* Unicasts or broadcasts a DHCP renew message */
+/* Unicast or broadcast a DHCP renew message */
int FAST_FUNC send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
{
struct dhcp_packet packet;
@@ -186,7 +186,7 @@ int FAST_FUNC send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr)
}
-/* Unicasts a DHCP release message */
+/* Unicast a DHCP release message */
int FAST_FUNC send_release(uint32_t server, uint32_t ciaddr)
{
struct dhcp_packet packet;
networking/udhcp/domain_codec.c+39 -34
@@ -25,16 +25,9 @@
*/
char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre)
{
- const uint8_t *c;
- int crtpos, retpos, depth, plen = 0, len = 0;
+ char *ret = ret; /* for compiler */
char *dst = NULL;
- if (!cstr)
- return NULL;
-
- if (pre)
- plen = strlen(pre);
-
/* We make two passes over the cstr string. First, we compute
* how long the resulting string would be. Then we allocate a
* new buffer of the required length, and fill it in with the
@@ -42,59 +35,71 @@ char* FAST_FUNC dname_dec(const uint8_t *cstr, int clen, const char *pre)
* having to deal with requiring callers to supply their own
* buffer, then having to check if it's sufficiently large, etc.
*/
-
- while (!dst) {
-
- if (len > 0) { /* second pass? allocate dst buffer and copy pre */
- dst = xmalloc(len + plen);
- memcpy(dst, pre, plen);
- }
+ while (1) {
+ /* note: "return NULL" below are leak-safe since
+ * dst isn't yet allocated */
+ const uint8_t *c;
+ unsigned crtpos, retpos, depth, len;
crtpos = retpos = depth = len = 0;
-
while (crtpos < clen) {
c = cstr + crtpos;
- if ((*c & NS_CMPRSFLGS) != 0) { /* pointer */
- if (crtpos + 2 > clen) /* no offset to jump to? abort */
+ if (*c & NS_CMPRSFLGS) {
+ /* pointer */
+ if (crtpos + 2 > clen) /* no offset to jump to? abort */
return NULL;
- if (retpos == 0) /* toplevel? save return spot */
+ if (retpos == 0) /* toplevel? save return spot */
retpos = crtpos + 2;
depth++;
- crtpos = ((*c & 0x3f) << 8) | (*(c + 1) & 0xff); /* jump */
- } else if (*c) { /* label */
- if (crtpos + *c + 1 > clen) /* label too long? abort */
+ crtpos = ((c[0] & 0x3f) << 8) | (c[1] & 0xff); /* jump */
+ } else if (*c) {
+ /* label */
+ if (crtpos + *c + 1 > clen) /* label too long? abort */
return NULL;
if (dst)
- memcpy(dst + plen + len, c + 1, *c);
+ memcpy(dst + len, c + 1, *c);
len += *c + 1;
crtpos += *c + 1;
if (dst)
- *(dst + plen + len - 1) = '.';
- } else { /* null: end of current domain name */
- if (retpos == 0) { /* toplevel? keep going */
+ dst[len - 1] = '.';
+ } else {
+ /* null: end of current domain name */
+ if (retpos == 0) {
+ /* toplevel? keep going */
crtpos++;
- } else { /* return to toplevel saved spot */
+ } else {
+ /* return to toplevel saved spot */
crtpos = retpos;
retpos = depth = 0;
}
if (dst)
- *(dst + plen + len - 1) = ' ';
+ dst[len - 1] = ' ';
}
- if (depth > NS_MAXDNSRCH || /* too many jumps? abort, it's a loop */
- len > NS_MAXDNAME * NS_MAXDNSRCH) /* result too long? abort */
+ if (depth > NS_MAXDNSRCH /* too many jumps? abort, it's a loop */
+ || len > NS_MAXDNAME * NS_MAXDNSRCH /* result too long? abort */
+ ) {
return NULL;
+ }
}
- if (!len) /* expanded string has 0 length? abort */
+ if (!len) /* expanded string has 0 length? abort */
return NULL;
- if (dst)
- *(dst + plen + len - 1) = '\0';
+ if (!dst) { /* first pass? */
+ /* allocate dst buffer and copy pre */
+ unsigned plen = strlen(pre);
+ ret = dst = xmalloc(plen + len);
+ memcpy(dst, pre, plen);
+ dst += plen;
+ } else {
+ dst[len - 1] = '\0';
+ break;
+ }
}
- return dst;
+ return ret;
}
/* Convert a domain name (src) from human-readable "foo.blah.com" format into
networking/udhcp/options.c+1 -1
@@ -115,7 +115,7 @@ const uint8_t dhcp_option_lengths[] ALIGN1 = {
[OPTION_U16] = 2,
[OPTION_S16] = 2,
[OPTION_U32] = 4,
- [OPTION_S32] = 4
+ [OPTION_S32] = 4,
};
networking/udhcp/options.h+5 -3
@@ -19,11 +19,13 @@ enum {
OPTION_U16,
OPTION_S16,
OPTION_U32,
- OPTION_S32
+ OPTION_S32,
};
-#define OPTION_REQ 0x10 /* have the client request this option */
-#define OPTION_LIST 0x20 /* There can be a list of 1 or more of these */
+/* Client requests this option by default */
+#define OPTION_REQ 0x10
+/* There can be a list of 1 or more of these */
+#define OPTION_LIST 0x20
/*****************************************************************/
/* Do not modify below here unless you know what you are doing!! */
networking/udhcp/script.c+19 -18
@@ -14,7 +14,7 @@
/* get a rough idea of how long an option will be (rounding up...) */
-static const uint8_t max_option_length[] = {
+static const uint8_t len_of_option_as_string[] = {
[OPTION_IP] = sizeof("255.255.255.255 "),
[OPTION_IP_PAIR] = sizeof("255.255.255.255 ") * 2,
[OPTION_STRING] = 1,
@@ -30,17 +30,10 @@ static const uint8_t max_option_length[] = {
};
-static inline int upper_length(int length, int opt_index)
-{
- return max_option_length[opt_index] *
- (length / dhcp_option_lengths[opt_index]);
-}
-
-
/* note: ip is a pointer to an IP in network order, possibly misaliged */
static int sprint_nip(char *dest, const char *pre, const uint8_t *ip)
{
- return sprintf(dest, "%s%d.%d.%d.%d", pre, ip[0], ip[1], ip[2], ip[3]);
+ return sprintf(dest, "%s%u.%u.%u.%u", pre, ip[0], ip[1], ip[2], ip[3]);
}
@@ -57,9 +50,10 @@ static int mton(uint32_t mask)
}
-/* Allocate and fill with the text of option 'option'. */
-static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p, const char *opt_name)
+/* Create "opt_name=opt_value" string */
+static char *xmalloc_optname_optval(uint8_t *option, const struct dhcp_option *type_p, const char *opt_name)
{
+ unsigned upper_length;
int len, type, optlen;
uint16_t val_u16;
int16_t val_s16;
@@ -67,14 +61,16 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p,
int32_t val_s32;
char *dest, *ret;
- len = option[OPT_LEN - 2];
+ /* option points to OPT_DATA, need to go back and get OPT_LEN */
+ len = option[OPT_LEN - OPT_DATA];
type = type_p->flags & TYPE_MASK;
optlen = dhcp_option_lengths[type];
+ upper_length = len_of_option_as_string[type] * (len / optlen);
- dest = ret = xmalloc(upper_length(len, type) + strlen(opt_name) + 2);
+ dest = ret = xmalloc(upper_length + strlen(opt_name) + 2);
dest += sprintf(ret, "%s=", opt_name);
- for (;;) {
+ while (len >= optlen) {
switch (type) {
case OPTION_IP_PAIR:
dest += sprint_nip(dest, "", option);
@@ -114,15 +110,20 @@ static char *alloc_fill_opts(uint8_t *option, const struct dhcp_option *type_p,
case OPTION_STR1035:
/* unpack option into dest; use ret for prefix (i.e., "optname=") */
dest = dname_dec(option, len, ret);
- free(ret);
- return dest;
+ if (dest) {
+ free(ret);
+ return dest;
+ }
+ /* error. return "optname=" string */
+ return ret;
#endif
}
option += optlen;
len -= optlen;
if (len <= 0)
break;
- dest += sprintf(dest, " ");
+ *dest++ = ' ';
+ *dest = '\0';
}
return ret;
}
@@ -174,7 +175,7 @@ static char **fill_envp(struct dhcp_packet *packet)
temp = get_option(packet, dhcp_options[i].code);
if (!temp)
goto next;
- *curr = alloc_fill_opts(temp, &dhcp_options[i], opt_name);
+ *curr = xmalloc_optname_optval(temp, &dhcp_options[i], opt_name);
putenv(*curr++);
/* Fill in a subnet bits option for things like /24 */