Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Many string command updates to help prevent buffer overflows. #72

Closed
wants to merge 6 commits into from
Closed

Many string command updates to help prevent buffer overflows. #72

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
24437a1
Many string command updates to help prevent buffer overflows.
Feb 18, 2018
f2a1077
Update strlcat/strlcpy to strncat/strncpy for portability. Had to bac…
Feb 18, 2018
5c83e0b
Convert strlcat to strncat
Feb 18, 2018
e0beb80
Fix missing parentheses
Feb 18, 2018
d6656ad
Fix whitespace/style
Feb 20, 2018
787235a
Fix strncpy lengths
Feb 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions a2s.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
server->max_players = (unsigned char)pkt[1];

// version
sprintf(buf, "%hhu", pkt[2]);
snprintf( buf, sizeof(buf), "%hhu", pkt[2] );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an example of the style we don't use. It should instead be:

snprintf(buf, sizeof(buf), "%hhu", pkt[2]);

add_rule(server, "version", buf, 0);

// dedicated
Expand Down Expand Up @@ -373,13 +373,13 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
}

// mod version
sprintf(buf, "%u", swap_long_from_little(pkt));
snprintf( buf, sizeof(buf), "%u", swap_long_from_little(pkt));
add_rule(server, "mod_ver", buf, 0);
pkt += 4;
pktlen -= 4;

// mod size
sprintf(buf, "%u", swap_long_from_little(pkt));
snprintf( buf, sizeof(buf), "%u", swap_long_from_little(pkt));
add_rule(server, "mod_size", buf, 0);
pkt += 4;
pktlen -= 4;
Expand All @@ -405,7 +405,7 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
pktlen--;

// Bots
sprintf(buf, "%hhu", *pkt);
snprintf( buf, sizeof(buf), "%hhu", *pkt );
add_rule(server, "bots", buf, 0);
pkt++;
pktlen--;
Expand Down Expand Up @@ -474,7 +474,7 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
server->num_players = (unsigned char)pkt[2];
server->max_players = (unsigned char)pkt[3];
// pkt[4] number of bots
sprintf(buf, "%hhu", pkt[4]);
snprintf( buf, sizeof(buf), "%hhu", pkt[4] );
add_rule(server, "bots", buf, 0);

add_rule(server, "dedicated", pkt[5] ? "1" : "0", 0);
Expand Down Expand Up @@ -524,7 +524,7 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
goto out_too_short;
}
gameport = swap_short_from_little(pkt);
sprintf(buf, "%hu", gameport);
snprintf( buf, sizeof(buf), "%hu", gameport );
add_rule(server, "game_port", buf, 0);
change_server_port(server, gameport, 0);
pkt += 2;
Expand All @@ -547,7 +547,7 @@ deal_with_a2s_packet(struct qserver *server, char *rawpkt, int pktlen)
goto out_too_short;
}
spectator_port = swap_short_from_little(pkt);
sprintf(buf, "%hu", spectator_port);
snprintf( buf, sizeof(buf), "%hu", spectator_port );
add_rule(server, "spectator_port", buf, 0);
pkt += 2;
pktlen -= 2;
Expand Down
17 changes: 10 additions & 7 deletions config.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,8 @@ qsc_load_default_config_files()
}
strncpy(path, var, len);
path[len] = '\0';
strcat(path, "/");
strcat(path, HOME_CONFIG_FILE);
strncat( path, "/", sizeof(path) - 1 - strlen(path));
strncat( path, HOME_CONFIG_FILE, sizeof(path) - 1 - strlen(path));
/* sprintf( path, "%s/%s", var, HOME_CONFIG_FILE); */
rc = try_load_config_file(path, 0);
if ((rc == 0) || (rc == -1)) {
Expand All @@ -210,14 +210,16 @@ qsc_load_default_config_files()
}

#ifdef sysconfdir
strcpy(path, sysconfdir "/qstat.cfg");
strncpy( path, sysconfdir "/qstat.cfg", sizeof(path) -1);
path[sizeof(path) -1] = '\0';
filename = path;
#elif defined(_WIN32)
if ((filename == NULL) && _pgmptr && strchr(_pgmptr, '\\')) {
char *slash = strrchr(_pgmptr, '\\');
strncpy(path, _pgmptr, slash - _pgmptr);
path[slash - _pgmptr] = '\0';
strcat(path, "\\qstat.cfg");
strncat( path, "\\qstat.cfg", sizeof(path) -1);
path[sizeof(path) -1] = '\0';
filename = path;
}
#endif
Expand Down Expand Up @@ -556,7 +558,8 @@ get_config_key(char *first_token, const ConfigKey *keys)
char key_name[1024], *token;
int key = 0;

strcpy(key_name, first_token);
strncpy( key_name, first_token, sizeof(key_name) -1);
key_name[sizeof(key_name) -1] = '\0';
do {
int k;
for (k = 0; keys[k].key_name; k++) {
Expand All @@ -576,8 +579,8 @@ get_config_key(char *first_token, const ConfigKey *keys)
REPORT_ERROR((stderr, "Key name too long"));
return (-1);
}
strcat(key_name, " ");
strcat(key_name, token);
strncat( key_name, " ", sizeof(key_name) - 1 - strlen(key_name));
strncat( key_name, token, sizeof(key_name) - 1 - strlen(key_name));
} while (1);

if (key == 0) {
Expand Down
10 changes: 5 additions & 5 deletions crysis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ send_crysis_request_packet(struct qserver *server)
case 0:
// Not seen a challenge yet, request it
server->challenge++;
sprintf(cmd, "challenge");
snprintf( cmd, sizeof(cmd), "challenge" );
break;

case 1:
server->challenge++;
password = get_param_value(server, "password", "");
sprintf(cmd, "%s:%s", server->challenge_string, password);
snprintf( cmd, sizeof(cmd), "%s:%s", server->challenge_string, password );
md5 = md5_hex(cmd, strlen(cmd));
sprintf(cmd, "authenticate %s", md5);
snprintf( cmd, sizeof(cmd), "authenticate %s", md5 );
free(md5);
break;

Expand All @@ -63,15 +63,15 @@ send_crysis_request_packet(struct qserver *server)
server->challenge++;
server->flags |= TF_STATUS_QUERY;
server->n_servers = 3;
sprintf(cmd, "status");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buffer is correctly sized

snprintf( cmd, sizeof(cmd), "status" );
break;

case 3:
return (DONE_FORCE);
}

server->saved_data.pkt_max = -1;
sprintf(buf, "POST /RPC2 HTTP/1.1\015\012Keep-Alive: 300\015\012User-Agent: qstat %s\015\012Content-Length: %d\015\012Content-Type: text/xml\015\012\015\012<?xml version=\"1.0\" encoding=\"UTF-8\"?><methodCall><methodName>%s</methodName><params /></methodCall>", VERSION, (int)(98 + strlen(cmd)), cmd);
snprintf(buf, sizeof(buf), "POST /RPC2 HTTP/1.1\015\012Keep-Alive: 300\015\012User-Agent: qstat %s\015\012Content-Length: %d\015\012Content-Type: text/xml\015\012\015\012<?xml version=\"1.0\" encoding=\"UTF-8\"?><methodCall><methodName>%s</methodName><params /></methodCall>", VERSION, (int)(98 + strlen(cmd)), cmd);

return (send_packet(server, buf, strlen(buf)));
}
Expand Down
20 changes: 10 additions & 10 deletions cube2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,43 +179,43 @@ deal_with_cube2_packet(struct qserver *server, char *rawpkt, int pktlen)

server->protocol_version = attr[0];

sprintf(buf, "%d %s", attr[0], sb_getversion_s(attr[0]));
snprintf( buf, sizeof(buf), "%d %s", attr[0], sb_getversion_s (attr[0]) );
add_rule(server, "version", buf, NO_FLAGS);

sprintf(buf, "%d %s", attr[1], sb_getmode_s(attr[1]));
snprintf( buf, sizeof(buf), "%d %s", attr[1], sb_getmode_s (attr[1]) );
add_rule(server, "mode", buf, NO_FLAGS);

sprintf(buf, "%d", attr[2]);
snprintf( buf, sizeof(buf), "%d", attr[2] );
add_rule(server, "seconds_left", buf, NO_FLAGS);

server->max_players = attr[3];

switch (attr[5]) {
case MM_OPEN:
sprintf(buf, "%d open", attr[5]);
snprintf( buf, sizeof(buf), "%d open", attr[5] );
break;

case MM_VETO:
sprintf(buf, "%d veto", attr[5]);
snprintf( buf, sizeof(buf), "%d veto", attr[5] );
break;

case MM_LOCKED:
sprintf(buf, "%d locked", attr[5]);
snprintf( buf, sizeof(buf), "%d locked", attr[5] );
break;

case MM_PRIVATE:
sprintf(buf, "%d private", attr[5]);
snprintf( buf, sizeof(buf), "%d private", attr[5] );
break;

default:
sprintf(buf, "%d unknown", attr[5]);
snprintf( buf, sizeof(buf), "%d unknown", attr[5] );
}
add_rule(server, "mm", buf, NO_FLAGS);

for (i = 0; i < numattr && i < MAX_ATTR; i++) {
char buf2[MAX_STRING];
sprintf(buf, "attr%d", i);
sprintf(buf2, "%d", attr[i]);
snprintf( buf, sizeof(buf), "attr%d", i );
snprintf( buf2, sizeof(buf2), "%d", attr[i] );
add_rule(server, buf, buf2, NO_FLAGS);
}

Expand Down
4 changes: 2 additions & 2 deletions debug.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ malformed_packet(const struct qserver *server, const char *fmt, ...)
char fn[PATH_MAX] = { 0 };
int fd;

sprintf(fn, "%03u_%s.pkt", count++, tag);
snprintf(fn, sizeof(fn), "%03u_%s.pkt", count++, tag);
fprintf(stderr, "dumping to %s\n", fn);
fd = open(fn, O_WRONLY | O_CREAT | O_EXCL, 0644);
if (fd == -1) {
Expand Down Expand Up @@ -171,7 +171,7 @@ output_packet(struct qserver *server, const char *buf, int buflen, int to)
for (i = buflen; i; offset += 16) {
memset(line, ' ', 256);
h = 0;
h += sprintf(line, "%5d:", offset);
h+= snprintf( line, sizeof(line), "%5d:", offset);
a = astart = h + 16 * 2 + 16 / 4 + 2;
for (b = 16; b && i; b--, i--, p++) {
if ((b & 3) == 0) {
Expand Down
2 changes: 1 addition & 1 deletion dirtybomb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ send_dirtybomb_request_packet(struct qserver *server)
chunks |= 0x04; // Player
chunks |= 0x08; // Team - Currently not supported
}
sprintf(buf, "%c%c%s%c", 0x01, len, password, chunks);
snprintf(buf, sizeof(buf), "%c%c%s%c", 0x01, len, password, chunks);

server->saved_data.pkt_max = -1;

Expand Down
2 changes: 1 addition & 1 deletion doom3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ _deal_with_doom3_packet(struct qserver *server, char *rawpkt, int pktlen, unsign
server->max_players = atoi(val);
} else if (0 == strcasecmp(key, "ri_maxViewers")) {
char max[20];
sprintf(max, "%d", server->max_players);
snprintf( max, sizeof(max), "%d", server->max_players );
add_rule(server, "si_maxplayers", max, NO_FLAGS);
server->max_players = atoi(val);
} else if (0 == strcasecmp(key, "ri_numViewers")) {
Expand Down
2 changes: 1 addition & 1 deletion farmsim.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ send_farmsim_request_packet(struct qserver *server)

server->saved_data.pkt_max = -1;
code = get_param_value(server, "code", "");
sprintf(buf, "GET /feed/dedicated-server-stats.xml?code=%s HTTP/1.1\015\012User-Agent: qstat\015\012\015\012", code);
snprintf(buf, sizeof(buf), "GET /feed/dedicated-server-stats.xml?code=%s HTTP/1.1\015\012User-Agent: qstat\015\012\015\012", code);

return (send_packet(server, buf, strlen(buf)));
}
Expand Down
8 changes: 4 additions & 4 deletions fl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,20 +340,20 @@ deal_with_fl_packet(struct qserver *server, char *rawpkt, int pktlen)
add_rule(server, "passworded", (*pkt++) ? "1" : "0", 0);

// FrameTime
sprintf(buf, "%hhu", *pkt++);
snprintf( buf, sizeof(buf), "%hhu", *pkt++ );
add_rule(server, "frametime", buf, 0);

// Round
sprintf(buf, "%hhu", *pkt++);
snprintf( buf, sizeof(buf), "%hhu", *pkt++ );
add_rule(server, "round", buf, 0);

// RoundMax
sprintf(buf, "%hhu", *pkt++);
snprintf( buf, sizeof(buf), "%hhu", *pkt++ );
add_rule(server, "roundmax", buf, 0);

// RoundSeconds
tmp_short = ((unsigned short)pkt[0] << 8) | ((unsigned short)pkt[1]);
sprintf(buf, "%hu", tmp_short);
snprintf( buf, sizeof(buf), "%hhu", tmp_short );
add_rule(server, "roundseconds", buf, 0);
pkt += 2;

Expand Down
16 changes: 9 additions & 7 deletions gs3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,8 @@ deal_with_gs3_status(struct qserver *server, char *rawpkt, int pktlen)
if (server->server_name) {
char *name = (char *)realloc(server->server_name, strlen(server->server_name) + strlen(val) + 3);
if (name) {
strcat(name, ": ");
strcat(name, val);
strncat( name, ": ", sizeof(name) - 1 - strlen(name) );
strncat( name, val, sizeof(name) - 1 - strlen(name) );
server->server_name = name;
}
}
Expand Down Expand Up @@ -381,8 +381,8 @@ process_gs3_packet(struct qserver *server)
if (server->server_name) {
char *name = (char *)realloc(server->server_name, strlen(server->server_name) + strlen(val) + 3);
if (name) {
strcat(name, ": ");
strcat(name, val);
strncat( name, ": ", sizeof(name) - 1 - strlen(name) );
strncat( name, val, sizeof(name) - 1 - strlen(name) );
server->server_name = name;
}
}
Expand Down Expand Up @@ -643,7 +643,7 @@ process_gs3_packet(struct qserver *server)
case TEAM_OTHER_HEADER:
default:
// add as a server rule
sprintf(rule, "%s%d", header, total_teams);
snprintf( rule, sizeof(rule), "%s%d", header, total_teams );
add_rule(server, rule, val, NO_FLAGS);
break;
}
Expand Down Expand Up @@ -675,8 +675,9 @@ send_gs3_request_packet(struct qserver *server)
server->flags |= TF_PLAYER_QUERY | TF_RULES_QUERY;
if (server->challenge) {
// we've recieved a challenge response, send the query + challenge id
len = sprintf(
len = snprintf(
query_buf,
sizeof(query_buf),
"\xfe\xfd%c\x10\x20\x30\x40%c%c%c%c\xff\xff\xff\x01",
0x00,
(unsigned char)(server->challenge >> 24),
Expand All @@ -694,8 +695,9 @@ send_gs3_request_packet(struct qserver *server)
server->flags |= TF_STATUS_QUERY;
if (server->challenge) {
// we've recieved a challenge response, send the query + challenge id
len = sprintf(
len = snprintf(
query_buf,
sizeof(query_buf),
"\xfe\xfd%c\x10\x20\x30\x40%c%c%c%c\x06\x01\x06\x05\x08\x0a\x04%c%c",
0x00,
(unsigned char)(server->challenge >> 24),
Expand Down
5 changes: 3 additions & 2 deletions haze.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ process_haze_packet(struct qserver *server)
case TEAM_OTHER_HEADER:
default:
// add as a server rule
sprintf(rule, "%s%d", header, total_teams);
snprintf( rule, sizeof(rule), "%s%d", header, total_teams );
add_rule(server, rule, val, NO_FLAGS);
break;
}
Expand Down Expand Up @@ -533,8 +533,9 @@ send_haze_request_packet(struct qserver *server)

if (server->challenge) {
// we've recieved a challenge response, send the query + challenge id
len = sprintf(
len = snprintf(
query_buf,
sizeof(query_buf),
"frdquery%c%c%c%c%c",
(unsigned char)(server->challenge >> 24),
(unsigned char)(server->challenge >> 16),
Expand Down
4 changes: 2 additions & 2 deletions mumble.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ deal_with_mumble_packet(struct qserver *server, char *rawpkt, int pktlen)

// version
server->protocol_version = ntohl(*(unsigned long *)pkt);
sprintf(version, "%u.%u.%u", (unsigned char)*pkt + 1, (unsigned char)*pkt + 2, (unsigned char)*pkt + 3);
snprintf(version,sizeof(version),"%u.%u.%u", (unsigned char)*pkt+1, (unsigned char)*pkt+2, (unsigned char)*pkt+3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing space after comma and removal of spaces before and after +

add_rule(server, "version", version, NO_FLAGS);
pkt += 4;

Expand All @@ -59,7 +59,7 @@ deal_with_mumble_packet(struct qserver *server, char *rawpkt, int pktlen)
pkt += 4;

// allowed bandwidth
sprintf(bandwidth, "%d", ntohl(*(unsigned long *)pkt));
snprintf( bandwidth, sizeof(bandwidth), "%d", ntohl(*(unsigned long*)pkt));
add_rule(server, "allowed_bandwidth", bandwidth, NO_FLAGS);
pkt += 4;

Expand Down
Loading