From 79bbf6f758ecf74e9eae86b12832adfe6fcc0a48 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 23 Sep 2012 21:15:34 +0200 Subject: [PATCH] symbols: avoid possible access-out-of-bound due to explicit_group The code that handles group name statements currently does this: info->group_names[grp - 1 + info->explicit_group] = name; Other than the fact that this addition makes no sense, it actually can reach out of the bounds of the array (which is of size XKB_NUM_GROUPS) in the (non-realistic) case where (grp - 1) is not 0 (i.e. the statement is not name[Group1] = "foo"). We also change explicit_group to be XKB_LAYOUT_INVALID if not set otherwise, instead of initializing it to 0; this is clearer and if someone happens to write 'us:1' for some reason, it will discard the other groups in the file as it should. This entire explicit_group thing was clearly bolted on as an afterthought. Signed-off-by: Ran Benita --- src/xkbcomp/symbols.c | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c index a412495..05cf10e 100644 --- a/src/xkbcomp/symbols.c +++ b/src/xkbcomp/symbols.c @@ -184,6 +184,7 @@ InitSymbolsInfo(SymbolsInfo *info, struct xkb_keymap *keymap, InitKeyInfo(&info->dflt, file_id); InitVModInfo(&info->vmods, keymap); info->actions = actions; + info->explicit_group = XKB_LAYOUT_INVALID; } static void @@ -1056,7 +1057,18 @@ SetGroupName(SymbolsInfo *info, ExprDef *arrayNdx, ExprDef *value) return false; } - info->group_names[grp - 1 + info->explicit_group] = name; + if (info->explicit_group == XKB_LAYOUT_INVALID) + info->group_names[grp - 1] = name; + else if (grp - 1 == 0) + info->group_names[info->explicit_group] = name; + else + log_warn(info->keymap->ctx, + "An explicit group was specified for the '%s' map, " + "but it provides a name for a group other than Group1 (%d); " + "Ignoring group name '%s'\n", + info->name, grp, + xkb_atom_text(info->keymap->ctx, name)); + return true; } @@ -1151,7 +1163,7 @@ SetExplicitGroup(SymbolsInfo *info, KeyInfo *keyi) GroupInfo *groupi; bool warn = false; - if (info->explicit_group == 0) + if (info->explicit_group == XKB_LAYOUT_INVALID) return true; darray_enumerate_from(i, groupi, keyi->groups, 1) { @@ -1169,9 +1181,12 @@ SetExplicitGroup(SymbolsInfo *info, KeyInfo *keyi) info->name, LongKeyNameText(keyi->name)); darray_resize0(keyi->groups, info->explicit_group + 1); - darray_item(keyi->groups, info->explicit_group) = - darray_item(keyi->groups, 0); - InitGroupInfo(&darray_item(keyi->groups, 0)); + if (info->explicit_group > 0) { + darray_item(keyi->groups, info->explicit_group) = + darray_item(keyi->groups, 0); + InitGroupInfo(&darray_item(keyi->groups, 0)); + } + return true; } -- 2.7.4