From 4a16503ea5ee05a84336d14dcf25d315780372b5 Mon Sep 17 00:00:00 2001 From: Matus Harvan Date: Sun, 26 Aug 2007 15:47:32 +0000 Subject: [PATCH] buffer overflow in dns.c pointed out by Matus Harvan, also strncpy cleanups --- src/common.h | 4 +++- src/dns.c | 13 +++++++------ src/iodine.c | 4 ++-- src/iodined.c | 4 ++-- src/tun.c | 17 ++++++++++------- src/tun.h | 6 +++--- 6 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/common.h b/src/common.h index f88d1d5..811aa40 100644 --- a/src/common.h +++ b/src/common.h @@ -28,6 +28,8 @@ #define MAX(a,b) ((a)>(b)?(a):(b)) #endif +#define QUERY_NAME_SIZE 256 + struct packet { int len; /* Total packet length */ @@ -37,7 +39,7 @@ struct packet }; struct query { - char name[258]; + char name[QUERY_NAME_SIZE]; short type; short id; struct sockaddr from; diff --git a/src/dns.c b/src/dns.c index 43bf346..1b6dd60 100644 --- a/src/dns.c +++ b/src/dns.c @@ -61,7 +61,7 @@ dns_encode(char *buf, size_t buflen, struct query *q, qr_t qr, char *data, size_ name = 0xc000 | ((p - buf) & 0x3fff); - putname(&p, 256, q->name); + putname(&p, sizeof(q->name), q->name); putshort(&p, q->type); putshort(&p, C_IN); @@ -78,7 +78,7 @@ dns_encode(char *buf, size_t buflen, struct query *q, qr_t qr, char *data, size_ header->qdcount = htons(1); header->arcount = htons(1); - putname(&p, 256, data); + putname(&p, datalen, data); putshort(&p, q->type); putshort(&p, C_IN); @@ -101,11 +101,11 @@ dns_encode(char *buf, size_t buflen, struct query *q, qr_t qr, char *data, size_ int dns_decode(char *buf, size_t buflen, struct query *q, qr_t qr, char *packet, size_t packetlen) { + char name[QUERY_NAME_SIZE]; char rdata[4*1024]; HEADER *header; short qdcount; short ancount; - char name[255]; uint32_t ttl; short class; short type; @@ -189,8 +189,8 @@ dns_decode(char *buf, size_t buflen, struct query *q, qr_t qr, char *packet, siz return -1; } - readname(packet, packetlen, &data, name, sizeof(name) -1); - name[256] = 0; + readname(packet, packetlen, &data, name, sizeof(name) - 1); + name[sizeof(name)-1] = '\0'; readshort(packet, &data, &type); readshort(packet, &data, &class); @@ -199,7 +199,8 @@ dns_decode(char *buf, size_t buflen, struct query *q, qr_t qr, char *packet, siz break; } - strncpy(q->name, name, 257); + strncpy(q->name, name, sizeof(q->name)); + q->name[sizeof(q->name) - 1] = '\0'; q->type = type; q->id = id; diff --git a/src/iodine.c b/src/iodine.c index 2476c97..5496b60 100644 --- a/src/iodine.c +++ b/src/iodine.c @@ -654,8 +654,8 @@ main(int argc, char **argv) device = optarg; break; case 'P': - strncpy(password, optarg, 32); - password[32] = 0; + strncpy(password, optarg, sizeof(password)); + password[sizeof(password)-1] = 0; /* XXX: find better way of cleaning up ps(1) */ memset(optarg, 0, strlen(optarg)); diff --git a/src/iodined.c b/src/iodined.c index 3a1cb76..5de3ceb 100644 --- a/src/iodined.c +++ b/src/iodined.c @@ -495,8 +495,8 @@ main(int argc, char **argv) } break; case 'P': - strncpy(password, optarg, 32); - password[32] = 0; + strncpy(password, optarg, sizeof(password)); + password[sizeof(password)-1] = 0; /* XXX: find better way of cleaning up ps(1) */ memset(optarg, 0, strlen(optarg)); diff --git a/src/tun.c b/src/tun.c index a8ce4e5..1453e47 100644 --- a/src/tun.c +++ b/src/tun.c @@ -58,7 +58,9 @@ open_tun(const char *tun_device) if (tun_device != NULL) { strncpy(ifreq.ifr_name, tun_device, IFNAMSIZ); + ifreq.ifr_name[IFNAMSIZ-1] = '\0'; strncpy(if_name, tun_device, sizeof(if_name)); + if_name[sizeof(if_name)-1] = '\0'; if (ioctl(tun_fd, TUNSETIFF, (void *) &ifreq) != -1) { printf("Opened %s\n", ifreq.ifr_name); @@ -102,6 +104,7 @@ open_tun(const char *tun_device) if (tun_device != NULL) { snprintf(tun_name, sizeof(tun_name), "/dev/%s", tun_device); strncpy(if_name, tun_device, sizeof(if_name)); + if_name[sizeof(if_name)-1] = '\0'; if ((tun_fd = open(tun_name, O_RDWR)) < 0) { warn("open_tun: %s: %s", tun_name, strerror(errno)); @@ -140,7 +143,7 @@ close_tun(int tun_fd) } int -write_tun(int tun_fd, char *data, int len) +write_tun(int tun_fd, char *data, size_t len) { #if defined (FREEBSD) || defined (DARWIN) || defined(NETBSD) data += 4; @@ -166,8 +169,8 @@ write_tun(int tun_fd, char *data, int len) return 0; } -int -read_tun(int tun_fd, char *buf, int len) +ssize_t +read_tun(int tun_fd, char *buf, size_t len) { #if defined (FREEBSD) || defined (DARWIN) || defined(NETBSD) /* FreeBSD/Darwin/NetBSD has no header */ @@ -213,20 +216,20 @@ tun_setip(const char *ip) } int -tun_setmtu(const int mtu) +tun_setmtu(const size_t mtu) { char cmdline[512]; if (mtu > 200 && mtu < 1500) { snprintf(cmdline, sizeof(cmdline), - "/sbin/ifconfig %s mtu %d", + "/sbin/ifconfig %s mtu %ld", if_name, mtu); - printf("Setting MTU of %s to %d\n", if_name, mtu); + printf("Setting MTU of %s to %ld\n", if_name, mtu); return system(cmdline); } else { - warn("MTU out of range: %d\n", mtu); + warn("MTU out of range: %ld\n", mtu); } return 1; diff --git a/src/tun.h b/src/tun.h index 2313b3a..72773de 100644 --- a/src/tun.h +++ b/src/tun.h @@ -19,9 +19,9 @@ int open_tun(const char *); void close_tun(int); -int write_tun(int, char *, int); -int read_tun(int, char *, int); +int write_tun(int, char *, size_t); +ssize_t read_tun(int, char *, size_t); int tun_setip(const char *); -int tun_setmtu(const int); +int tun_setmtu(const size_t); #endif /* _TUN_H_ */