I spent this afternoon tracking down some crash bugs, so i figured i would
save others the time looking for the same thing.
When you find bugs, please send them to the mailer and Jeremy. It will
save a lot of people time reproducing your effort, and help get Circle 3.0
released sooner and more stable.
1. There is a bug in string replacement (^). If you replace a substring of a
string with a very long string, the resulting string can be greater than
MAX_INPUT_LENGTH, which will usually result in a segmentation fault.
The fix is to use strncat in perform_subst() instead of strcat.
The fixed function:
/* perform substitution for the '^..^' csh-esque syntax */
int perform_subst(struct descriptor_data * t, char *orig, char *subst)
{
char new[MAX_INPUT_LENGTH + 5];
char *first, *second, *strpos;
first = subst + 1;
if (!(second = strchr(first, '^'))) {
SEND_TO_Q("Invalid substitution.\r\n", t);
return 1;
}
*(second++) = '\0';
if (!(strpos = strstr(orig, first))) {
SEND_TO_Q("Invalid substitution.\r\n", t);
return 1;
}
strncpy(new, orig, (strpos - orig));
new[(strpos - orig)] = '\0';
+ strncat(new, second, (MAX_INPUT_LENGTH - strlen(new) - 1));
if (((strpos - orig) + strlen(first)) < strlen(orig))
+ strncat(new, strpos + strlen(first), (MAX_INPUT_LENGTH - strlen(new) - 1));
strcpy(subst, new);
return 0;
}
2. A similar bug is in aliases. You can make an alias exceed MAX_INPUT_LENGTH
by using a long substitution string in a long alias, such as:
alias spam say spam $* spam spam spam.....
then
spam spam spam spam spam spam.....
This results in a seg fault too.
The fix is to shorten the string before adding it to the input queue:
void perform_complex_alias(struct txt_q *input_q, char *orig, struct alias *a)
{
struct txt_q temp_queue;
char *tokens[NUM_TOKENS], *temp, *write_point;
int num_of_tokens = 0, num;
/* First, parse the original string */
temp = strtok(strcpy(buf2, orig), " ");
while (temp != NULL && num_of_tokens < NUM_TOKENS) {
tokens[num_of_tokens++] = temp;
temp = strtok(NULL, " ");
}
/* initialize */
write_point = buf;
temp_queue.head = temp_queue.tail = NULL;
/* now parse the alias */
for (temp = a->replacement; *temp; temp++) {
if (*temp == ALIAS_SEP_CHAR) {
*write_point = '\0';
+ if (strlen(buf) > (MAX_INPUT_LENGTH - 1))
+ buf[MAX_INPUT_LENGTH - 1] = '\0';
write_to_q(buf, &temp_queue, 1);
write_point = buf;
} else if (*temp == ALIAS_VAR_CHAR) {
temp++;
if ((num = *temp - '1') < num_of_tokens && num >= 0) {
strcpy(write_point, tokens[num]);
write_point += strlen(tokens[num]);
} else if (*temp == ALIAS_GLOB_CHAR) {
strcpy(write_point, orig);
write_point += strlen(orig);
} else
if ((*(write_point++) = *temp) == '$') /* redouble $ for act safety */
*(write_point++) = '$';
} else
*(write_point++) = *temp;
}
*write_point = '\0';
+ if (strlen(buf) > (MAX_INPUT_LENGTH - 1))
+ buf[MAX_INPUT_LENGTH - 1] = '\0';
write_to_q(buf, &temp_queue, 1);
/* push our temp_queue on to the _front_ of the input queue */
if (input_q->head == NULL)
*input_q = temp_queue;
else {
temp_queue.tail->next = input_q->head;
input_q->head = temp_queue.head;
}
}
3. There is a bug in nanny, causing a crash if someone tries to log in with
a very long name. The problem is in _parse_name(), a name longer than 20
characters gets writen off the end of tmp_name. The fix is just to switch
around the MAX_NAME_LENGTH check and the _parse_name. tmp_name should also be
define as tmp_name[MAX_NAME_LENGTH] instead of tmp_name[20].
Old code from nanny():
- if ((_parse_name(arg, tmp_name)) || strlen(tmp_name) < 2 ||
- strlen(tmp_name) > MAX_NAME_LENGTH ||
fill_word(strcpy(buf, tmp_name)) || reserved_word(buf)) {
SEND_TO_Q("Invalid name, please try another.\r\n"
"Name: ", d);
return;
}
Fix
+ if (strlen(tmp_name) > MAX_NAME_LENGTH ||
+ (_parse_name(arg, tmp_name)) || strlen(tmp_name) < 2 ||
fill_word(strcpy(buf, tmp_name)) || reserved_word(buf)) {
SEND_TO_Q("Invalid name, please try another.\r\n"
"Name: ", d);
return;
}
4. There is a bug in do_users() and do_who(), which can cause crashes if
someone uses 'who -n <really long name>'. The name_search[80] and (for
do_users only) host_search[80] should be arrays of size MAX_STRING_LENGTH, not
80.
This last bug was found by others (i was told by Dert of Seg Fault, and also
heard it from Guru of Hex), but i included it in case it hasn't been reported
yet, or someone had missed it.
Eric Green
ejg3@cornell.edu
Stefan Wasilewski
smwasice@vivarin.pc.cc.cmu.edu
This archive was generated by hypermail 2b30 : 12/07/00 PST