ircd: substitution: fix buffer overrun if variable name is too long

Also fix the "ptr = ptr + (pptr - ptr)" aka "ptr = pptr" mess by
removing pptr.
This commit is contained in:
Simon Arlott 2017-08-05 11:27:02 +01:00
parent 6b80043eca
commit 4212494106
No known key found for this signature in database
GPG key ID: C8975F2043CA5D24
2 changed files with 33 additions and 27 deletions

View file

@ -101,45 +101,30 @@ char *substitution_parse(const char *fmt, rb_dlink_list *varlist)
const char *ptr; const char *ptr;
char *bptr = buf; char *bptr = buf;
for (ptr = fmt; *ptr != '\0' && bptr - buf < BUFSIZE; ptr++) for (ptr = fmt; *ptr != '\0' && bptr - buf < BUFSIZE; ptr++) {
if (*ptr != '$') if (*ptr != '$') {
*bptr++ = *ptr; *bptr++ = *ptr;
else if (*(ptr + 1) == '{') } else if (*(ptr + 1) == '{') {
{ char varname[BUFSIZE] = { 0 };
static char varname[BUFSIZE];
char *vptr = varname; char *vptr = varname;
const char *pptr;
rb_dlink_node *nptr; rb_dlink_node *nptr;
*vptr = '\0';
/* break out ${var} */ /* break out ${var} */
for (pptr = ptr + 2; *pptr != '\0'; pptr++) for (ptr += 2; *ptr != '\0'; ptr++) {
{ if (*ptr == '$') {
if (*pptr == '$') ptr--;
{
*vptr++ = '\0';
pptr--;
break; break;
} } else if (*ptr == '}') {
else if (*pptr != '}')
*vptr++ = *pptr;
else
{
*vptr++ = '\0';
break; break;
} else if (vptr < &varname[sizeof(varname) - 1]) {
*vptr++ = *ptr;
} }
} }
/* advance ptr by length of variable */ RB_DLINK_FOREACH(nptr, varlist->head) {
ptr += (pptr - ptr);
RB_DLINK_FOREACH(nptr, varlist->head)
{
struct substitution_variable *val = (struct substitution_variable *) nptr->data; struct substitution_variable *val = (struct substitution_variable *) nptr->data;
if (!rb_strcasecmp(varname, val->name)) if (!rb_strcasecmp(varname, val->name)) {
{
rb_strlcpy(bptr, val->value, sizeof(buf) - (bptr - buf)); rb_strlcpy(bptr, val->value, sizeof(buf) - (bptr - buf));
bptr += strlen(val->value); bptr += strlen(val->value);
if (bptr >= &buf[sizeof(buf)]) { if (bptr >= &buf[sizeof(buf)]) {
@ -153,6 +138,7 @@ char *substitution_parse(const char *fmt, rb_dlink_list *varlist)
if (*ptr == '\0') if (*ptr == '\0')
break; break;
} }
}
*bptr = '\0'; *bptr = '\0';
return buf; return buf;

View file

@ -270,6 +270,16 @@ static void too_long_variable3c(void) {
is_string(tmp2, substitution_parse("${text200}${text400}${text100}", &varlist2), MSG); is_string(tmp2, substitution_parse("${text200}${text400}${text100}", &varlist2), MSG);
} }
static void long_variable_name1(void) {
char input[2048];
strcpy(input, "${");
strcat(input, MKTEXT(1000));
strcat(input, "}");
is_string("test", substitution_parse(input, &varlist2), MSG);
}
int main(int argc, char *argv[]) int main(int argc, char *argv[])
{ {
memset(&me, 0, sizeof(me)); memset(&me, 0, sizeof(me));
@ -303,6 +313,11 @@ int main(int argc, char *argv[])
substitution_append_var(&varlist2, "text514", MKTEXT(514)); substitution_append_var(&varlist2, "text514", MKTEXT(514));
substitution_append_var(&varlist2, "text600", MKTEXT(600)); substitution_append_var(&varlist2, "text600", MKTEXT(600));
char temp[2048];
strcpy(temp, MKTEXT(1000));
temp[BUFSIZE - 1] = '\0';
substitution_append_var(&varlist2, temp, "test");
plan_lazy(); plan_lazy();
valid_variable1(); valid_variable1();
@ -336,5 +351,10 @@ int main(int argc, char *argv[])
too_long_variable3b(); too_long_variable3b();
too_long_variable3c(); too_long_variable3c();
long_variable_name1();
substitution_free(&varlist);
substitution_free(&varlist2);
return 0; return 0;
} }