Commit 79bbf6f758ecf74e9eae86b12832adfe6fcc0a48
2012-09-23T21:15:34
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 <ran234@gmail.com>
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57
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;
}