Fix a double-free on syntax error without a new line.

$ echo "#foo\nfoo" > custom_config $ X -config custom_config

will trigger the double free because the contents of xf86_lex_val.str
have been realloc()ed aready  when free is called in read.c:209.

This copies the lex token and adds all the necessary free() calls to
avoid leaking it

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1176>
This commit is contained in:
Matthieu Herrb 2023-10-14 19:06:22 +02:00 committed by Enrico Weigelt, metux IT consult
parent fd7da4bddd
commit bb36aa5ae1
17 changed files with 69 additions and 10 deletions

View File

@ -77,6 +77,8 @@ xf86parseDRISection(void)
break; break;
case COMMENT: case COMMENT:
ptr->dri_comment = xf86addComment(ptr->dri_comment, xf86_lex_val.str); ptr->dri_comment = xf86addComment(ptr->dri_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
default: default:
Error(INVALID_KEYWORD_MSG, xf86tokenString()); Error(INVALID_KEYWORD_MSG, xf86tokenString());

View File

@ -106,6 +106,8 @@ xf86parseDeviceSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->dev_comment = xf86addComment(ptr->dev_comment, xf86_lex_val.str); ptr->dev_comment = xf86addComment(ptr->dev_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->dev_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->dev_comment)) != XF86_TOKEN_STRING)

View File

@ -67,6 +67,8 @@ xf86parseExtensionsSection(void)
case COMMENT: case COMMENT:
ptr->extensions_comment = ptr->extensions_comment =
xf86addComment(ptr->extensions_comment, xf86_lex_val.str); xf86addComment(ptr->extensions_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
default: default:
Error(INVALID_KEYWORD_MSG, xf86tokenString()); Error(INVALID_KEYWORD_MSG, xf86tokenString());

View File

@ -89,6 +89,8 @@ xf86parseFilesSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->file_comment = xf86addComment(ptr->file_comment, xf86_lex_val.str); ptr->file_comment = xf86addComment(ptr->file_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case FONTPATH: case FONTPATH:
if (xf86getSubToken(&(ptr->file_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->file_comment)) != XF86_TOKEN_STRING)

View File

@ -98,6 +98,8 @@ xf86parseFlagsSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->flg_comment = xf86addComment(ptr->flg_comment, xf86_lex_val.str); ptr->flg_comment = xf86addComment(ptr->flg_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
/* /*
* these old keywords are turned into standard generic options. * these old keywords are turned into standard generic options.
@ -436,19 +438,25 @@ xf86parseOption(XF86OptionPtr head)
if ((token = xf86getSubToken(&comment)) == XF86_TOKEN_STRING) { if ((token = xf86getSubToken(&comment)) == XF86_TOKEN_STRING) {
option = xf86newOption(name, xf86_lex_val.str); option = xf86newOption(name, xf86_lex_val.str);
option->opt_comment = comment; option->opt_comment = comment;
if ((token = xf86getToken(NULL)) == COMMENT) if ((token = xf86getToken(NULL)) == COMMENT) {
option->opt_comment = xf86addComment(option->opt_comment, xf86_lex_val.str); option->opt_comment = xf86addComment(option->opt_comment, xf86_lex_val.str);
else free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
} else {
xf86unGetToken(token); xf86unGetToken(token);
} }
}
else { else {
option = xf86newOption(name, NULL); option = xf86newOption(name, NULL);
option->opt_comment = comment; option->opt_comment = comment;
if (token == COMMENT) if (token == COMMENT) {
option->opt_comment = xf86addComment(option->opt_comment, xf86_lex_val.str); option->opt_comment = xf86addComment(option->opt_comment, xf86_lex_val.str);
else free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
} else {
xf86unGetToken(token); xf86unGetToken(token);
} }
}
old = NULL; old = NULL;

View File

@ -84,6 +84,8 @@ xf86parseInputSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->inp_comment = xf86addComment(ptr->inp_comment, xf86_lex_val.str); ptr->inp_comment = xf86addComment(ptr->inp_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->inp_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->inp_comment)) != XF86_TOKEN_STRING)

View File

@ -191,6 +191,8 @@ xf86parseInputClassSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->comment = xf86addComment(ptr->comment, xf86_lex_val.str); ptr->comment = xf86addComment(ptr->comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->comment)) != XF86_TOKEN_STRING)

View File

@ -101,6 +101,8 @@ xf86parseLayoutSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->lay_comment = xf86addComment(ptr->lay_comment, xf86_lex_val.str); ptr->lay_comment = xf86addComment(ptr->lay_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->lay_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->lay_comment)) != XF86_TOKEN_STRING)

View File

@ -95,6 +95,8 @@ xf86parseModuleSubSection(XF86LoadPtr head, char *name)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->load_comment = xf86addComment(ptr->load_comment, xf86_lex_val.str); ptr->load_comment = xf86addComment(ptr->load_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case OPTION: case OPTION:
ptr->load_opt = xf86parseOption(ptr->load_opt); ptr->load_opt = xf86parseOption(ptr->load_opt);
@ -126,6 +128,8 @@ xf86parseModuleSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->mod_comment = xf86addComment(ptr->mod_comment, xf86_lex_val.str); ptr->mod_comment = xf86addComment(ptr->mod_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case LOAD: case LOAD:
if (xf86getSubToken(&(ptr->mod_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->mod_comment)) != XF86_TOKEN_STRING)
@ -230,10 +234,13 @@ xf86addNewLoadDirective(XF86LoadPtr head, const char *name, int type,
new->ignore = 0; new->ignore = 0;
new->list.next = NULL; new->list.next = NULL;
if ((token = xf86getToken(NULL)) == COMMENT) if ((token = xf86getToken(NULL)) == COMMENT) {
new->load_comment = xf86addComment(new->load_comment, xf86_lex_val.str); new->load_comment = xf86addComment(new->load_comment, xf86_lex_val.str);
else free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
} else {
xf86unGetToken(token); xf86unGetToken(token);
}
return ((XF86LoadPtr) xf86addListItem((glp) head, (glp) new)); return ((XF86LoadPtr) xf86addListItem((glp) head, (glp) new));
} }

View File

@ -269,6 +269,8 @@ xf86parseVerboseMode(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->ml_comment = xf86addComment(ptr->ml_comment, xf86_lex_val.str); ptr->ml_comment = xf86addComment(ptr->ml_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case DOTCLOCK: case DOTCLOCK:
if ((token = xf86getSubToken(&(ptr->ml_comment))) != NUMBER) if ((token = xf86getSubToken(&(ptr->ml_comment))) != NUMBER)
@ -413,6 +415,8 @@ xf86parseMonitorSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->mon_comment = xf86addComment(ptr->mon_comment, xf86_lex_val.str); ptr->mon_comment = xf86addComment(ptr->mon_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->mon_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->mon_comment)) != XF86_TOKEN_STRING)
@ -599,6 +603,8 @@ xf86parseModesSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->modes_comment = xf86addComment(ptr->modes_comment, xf86_lex_val.str); ptr->modes_comment = xf86addComment(ptr->modes_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->modes_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->modes_comment)) != XF86_TOKEN_STRING)

View File

@ -102,6 +102,8 @@ xf86parseOutputClassSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->comment = xf86addComment(ptr->comment, xf86_lex_val.str); ptr->comment = xf86addComment(ptr->comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->comment)) != XF86_TOKEN_STRING)

View File

@ -104,6 +104,8 @@ xf86parsePointerSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->inp_comment = xf86addComment(ptr->inp_comment, xf86_lex_val.str); ptr->inp_comment = xf86addComment(ptr->inp_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case PROTOCOL: case PROTOCOL:
if (xf86getSubToken(&(ptr->inp_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->inp_comment)) != XF86_TOKEN_STRING)

View File

@ -119,6 +119,8 @@ xf86parseDisplaySubSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->disp_comment = xf86addComment(ptr->disp_comment, xf86_lex_val.str); ptr->disp_comment = xf86addComment(ptr->disp_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case VIEWPORT: case VIEWPORT:
if (xf86getSubToken(&(ptr->disp_comment)) != NUMBER) if (xf86getSubToken(&(ptr->disp_comment)) != NUMBER)
@ -256,6 +258,8 @@ xf86parseScreenSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->scrn_comment = xf86addComment(ptr->scrn_comment, xf86_lex_val.str); ptr->scrn_comment = xf86addComment(ptr->scrn_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->scrn_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->scrn_comment)) != XF86_TOKEN_STRING)

View File

@ -98,6 +98,8 @@ xf86parseVendorSubSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->vs_comment = xf86addComment(ptr->vs_comment, xf86_lex_val.str); ptr->vs_comment = xf86addComment(ptr->vs_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->vs_comment))) if (xf86getSubToken(&(ptr->vs_comment)))
@ -151,6 +153,8 @@ xf86parseVendorSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->vnd_comment = xf86addComment(ptr->vnd_comment, xf86_lex_val.str); ptr->vnd_comment = xf86addComment(ptr->vnd_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->vnd_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->vnd_comment)) != XF86_TOKEN_STRING)

View File

@ -97,6 +97,8 @@ xf86parseVideoPortSubSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->vp_comment = xf86addComment(ptr->vp_comment, xf86_lex_val.str); ptr->vp_comment = xf86addComment(ptr->vp_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->vp_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->vp_comment)) != XF86_TOKEN_STRING)
@ -154,6 +156,8 @@ xf86parseVideoAdaptorSection(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->va_comment = xf86addComment(ptr->va_comment, xf86_lex_val.str); ptr->va_comment = xf86addComment(ptr->va_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case IDENTIFIER: case IDENTIFIER:
if (xf86getSubToken(&(ptr->va_comment)) != XF86_TOKEN_STRING) if (xf86getSubToken(&(ptr->va_comment)) != XF86_TOKEN_STRING)

View File

@ -100,6 +100,8 @@ xf86readConfigFile(void)
switch (token) { switch (token) {
case COMMENT: case COMMENT:
ptr->conf_comment = xf86addComment(ptr->conf_comment, xf86_lex_val.str); ptr->conf_comment = xf86addComment(ptr->conf_comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
break; break;
case SECTION: case SECTION:
if (xf86getSubToken(&(ptr->conf_comment)) != XF86_TOKEN_STRING) { if (xf86getSubToken(&(ptr->conf_comment)) != XF86_TOKEN_STRING) {

View File

@ -333,10 +333,10 @@ xf86getToken(const xf86ConfigSymTabRec * tab)
} }
while ((c != '\n') && (c != '\r') && (c != '\0')); while ((c != '\n') && (c != '\r') && (c != '\0'));
configRBuf[i] = '\0'; configRBuf[i] = '\0';
/* XXX no private copy. /* XXX private copy.
* Use xf86addComment when setting a comment. * Use xf86addComment when setting a comment.
*/ */
xf86_lex_val.str = configRBuf; xf86_lex_val.str = strdup(configRBuf);
return COMMENT; return COMMENT;
} }
@ -449,8 +449,11 @@ xf86getSubToken(char **comment)
for (;;) { for (;;) {
token = xf86getToken(NULL); token = xf86getToken(NULL);
if (token == COMMENT) { if (token == COMMENT) {
if (comment) if (comment) {
*comment = xf86addComment(*comment, xf86_lex_val.str); *comment = xf86addComment(*comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
}
} }
else else
return token; return token;
@ -465,8 +468,11 @@ xf86getSubTokenWithTab(char **comment, const xf86ConfigSymTabRec * tab)
for (;;) { for (;;) {
token = xf86getToken(tab); token = xf86getToken(tab);
if (token == COMMENT) { if (token == COMMENT) {
if (comment) if (comment) {
*comment = xf86addComment(*comment, xf86_lex_val.str); *comment = xf86addComment(*comment, xf86_lex_val.str);
free(xf86_lex_val.str);
xf86_lex_val.str = NULL;
}
} }
else else
return token; return token;