Commit d6f20014 authored by Kirk Webb's avatar Kirk Webb

Update sdcollectd to treat incoming report packets as poisonous.

sdcollectd has been changed to make the parsing of incoming idle report
packets more robust.  This should squash all existing vulnerabilities on
the packet reception path.

Changes:

* NULL terminate the incoming packet buffer
* move from strcpy to strncpy.
* switch from strtoul, strtod, etc. to sscanf with explicit field lengths.
* Multi-level parsing of packets into records, then key/val pairs,
  then individual key and value parsing/checking.
parent bf9f7f48
......@@ -99,7 +99,11 @@ int main(int argc, char **argv) {
bzero(&servaddr, sizeof(struct sockaddr_in));
servaddr.sin_family = AF_INET;
servaddr.sin_addr.s_addr = INADDR_ANY;
servaddr.sin_port = htons(SDCOLLECTD_PORT);
if (opts->port) {
servaddr.sin_port = htons(opts->port);
} else {
servaddr.sin_port = htons(SDCOLLECTD_PORT);
}
/* Create and bind udp socket for collecting slothd client-side idle data */
if ((sd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
......@@ -188,7 +192,8 @@ int main(int argc, char **argv) {
}
int CollectData(int sd, IDLE_DATA *iddata) {
int numbytes, slen;
int numbytes;
socklen_t slen;
time_t curtime;
/* struct hostent *hent; */
struct sockaddr_in cliaddr;
......@@ -210,6 +215,8 @@ int CollectData(int sd, IDLE_DATA *iddata) {
}
}
/* Null terminate the received buffer for safety. */
iddata->buf[sizeof(iddata->buf)-1] = '\0';
return 1;
}
......@@ -262,7 +269,8 @@ char *tbmac(char *maddr, char **endptr) {
mylast = ++myptr;
}
else if (myptr - maddr == MACADDRLEN) {
strcpy(tbaddr, maddr);
strncpy(tbaddr, maddr, MACADDRLEN);
tbaddr[MACADDRLEN] = '\0';
++myptr;
break;
}
......@@ -283,13 +291,16 @@ char *tbmac(char *maddr, char **endptr) {
int ParseRecord(IDLE_DATA *iddata) {
int tmpvers;
char *itemptr, *ptr, *tmpstr;
char *itemptr, *tmpstr;
int sres = 0;
char key[MAXKEYSIZE+1], value[MAXVALUESIZE+1];
unsigned long val1, val2;
double dval1, dval2, dval3;
char maddr[18];
iddata->ifcnt = 0;
/* Parse out fields */
/* First parsing pass: separate out the key/value pairs. */
itemptr = strtok(iddata->buf, " \t");
if (itemptr == NULL) {
error("No valid data; rejecting.");
......@@ -297,45 +308,96 @@ int ParseRecord(IDLE_DATA *iddata) {
}
do {
if (strstr(itemptr, "vers")) {
if ((tmpvers = strtoul(itemptr+5, NULL, 10)) > SDPROTOVERS) {
error("Unsupported protocol version; rejecting.");
return 0;
/* Second pass: split off the key and value */
sres = sscanf(itemptr, KVSCAN_FORMAT(MAXKEYSIZE, MAXVALUESIZE),
key, value);
if (sres == 2) {
/* Third pass: parse value according to key type. */
if (!strcmp(key,"vers")) {
sres = sscanf(value, LONG_FORMAT, &val1);
if (sres == 1) {
if (val1 > SDPROTOVERS) {
error("Unsupported protocol version; report rejected.");
return 0;
} else {
iddata->version = val1;
}
}
else {
error("Packet from node %s rejected: Bad data in version field: %s",
iddata->id, value);
return 0;
}
}
iddata->version = tmpvers;
}
else if (strstr(itemptr, "mis")) {
iddata->mis = atol(itemptr+4);
}
else if (strstr(itemptr, "lave")) {
iddata->l1m = strtod(itemptr+5, &ptr);
iddata->l5m = strtod(ptr+1, &ptr);
iddata->l15m = strtod(ptr+1, NULL);
}
else if (strstr(itemptr, "iface")) {
if (!(tmpstr = tbmac(itemptr+6, &ptr))) {
error("Malformed interface record for node %s encountered: %s",
iddata->id,
itemptr);
continue;
else if (!strcmp(key,"mis")) {
sres = sscanf(value, LONG_FORMAT, &val1);
if (sres == 1) {
iddata->mis = val1;
}
else {
error("Packet from node %s rejected: Bad data in mis field: %s",
iddata->id, value);
return 0;
}
}
strcpy(iddata->ifaces[iddata->ifcnt].mac, tmpstr);
iddata->ifaces[iddata->ifcnt].ipkts = strtoul(ptr+1, &ptr, 10);
iddata->ifaces[iddata->ifcnt].opkts = strtoul(ptr+1, NULL, 10);
iddata->ifcnt++;
}
else if (strstr(itemptr, "abits")) {
iddata->actbits = strtoul(itemptr+6, NULL, 16);
}
else if (!strcmp(key,"abits")) {
sres = sscanf(value, HEX_FORMAT, &val1);
if (sres == 1) {
iddata->actbits = val1;
}
else {
error("Packet from node %s rejected: Bad data in abits field: %s",
iddata->id, value);
return 0;
}
}
else if (!strcmp(key,"lave")) {
sres = sscanf(value, DBL_FORMAT "," DBL_FORMAT "," DBL_FORMAT ,
&dval1, &dval2, &dval3);
if (sres == 3) {
iddata->l1m = dval1;
iddata->l5m = dval2;
iddata->l15m = dval3;
} else {
error("Packet from node %s rejected: Bad data in lave field: %s",
iddata->id, value);
return 0;
}
}
else if (!strcmp(key,"iface")) {
sres = sscanf(value, MADDR_FORMAT "," LONG_FORMAT "," LONG_FORMAT ,
maddr, &val1, &val2);
if (sres == 3) {
tmpstr = tbmac(maddr, NULL);
if (tmpstr) {
strncpy(iddata->ifaces[iddata->ifcnt].mac, tmpstr, MACADDRLEN);
iddata->ifaces[iddata->ifcnt].mac[MACADDRLEN] = '\0';
iddata->ifaces[iddata->ifcnt].ipkts = val1;
iddata->ifaces[iddata->ifcnt].opkts = val2;
iddata->ifcnt++;
} else {
error("Malformed interface record for node %s encountered: %s",
iddata->id,
maddr);
continue;
}
}
else {
error("Packet from node %s rejected: Bad data in iface field: %s",
iddata->id, value);
return 0;
}
}
else {
error("Packet rejected from node %s: Unknown key: %s", iddata->id, key);
return 0;
}
}
else {
info("Unrecognized string in packet: %s", itemptr);
error("Malformed packet received from node %s; rejecting.", iddata->id);
return 0;
}
} while ((itemptr = strtok(NULL, " \t")) && iddata->ifcnt < MAXNUMIFACES);
return 1;
......@@ -350,7 +412,7 @@ void PutDBRecord(IDLE_DATA *iddata) {
char tmpstr[(NUMACTTYPES+1)*sizeof(curstamp)];
char *actstr[] = ACTSTRARRAY;
sprintf(curstamp, "FROM_UNIXTIME(%lu)", now);
sprintf(curstamp, "FROM_UNIXTIME(%lu)", (long unsigned int)now);
printf("now: %s\n", ctime(&now));
......@@ -376,7 +438,7 @@ void PutDBRecord(IDLE_DATA *iddata) {
}
printf("\n\n");
}
if (opts->popold) {
if (!mydb_update("INSERT INTO node_idlestats VALUES ('%s', FROM_UNIXTIME(%lu), FROM_UNIXTIME(%lu), %f, %f, %f)",
iddata->id,
......
......@@ -36,6 +36,17 @@
#define MACADDRLEN 12
#define RUNASUSER "nobody"
#define MAXKEYSIZE 10
#define MAXVALUESIZE 40
#define LONG_FORMAT "%20lu"
#define DBL_FORMAT "%20lf"
#define HEX_FORMAT "%8lx"
#define MADDR_FORMAT "%17[0-9a-fA-F:-]"
/* CPP wackiness */
#define KVSCAN_FORMAT(x,y) KVSCAN_FORMAT_INDIRECT(x,y)
#define KVSCAN_FORMAT_INDIRECT(x,y) "%" #x "[^=]=%" #y "s"
#define NUMACTTYPES 4
#define ACTSTRARRAY {"last_tty_act", "last_cpu_act", "last_net_act", "last_ext_act"}
......@@ -57,8 +68,8 @@ typedef struct {
u_int version;
struct {
char mac[MACADDRLEN+1];
long ipkts;
long opkts;
unsigned long ipkts;
unsigned long opkts;
} ifaces[MAXNUMIFACES];
char id[NODENAMESIZE]; /* Host identifier - probably local part of hostname */
char buf[BUFSIZE]; /* String containing monitor output values */
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment