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>

diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index a412495..05cf10e 100644 (file)
--- 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;
 }