symbols: fix buffer overflow with illegal explicit group

Trying ''./test/interactive -l us:5' causes us to crash.

The <layout>:<N> syntax says to put this layout at the N'th level.
However the code (inherited from xkbcomp) doesn't check that the group
is valid, and then happily indexes keyi->groups with it, which has a
static size of XKB_NUM_GROUPS (the SetExplicitGroup function assumes the
index is valid). So any value a user might put there > 4 makes nice
things happen.

Signed-off-by: Ran Benita <ran234@gmail.com>

diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index 9c4fb1c9c39d67f414d999c755d96a6ecc6b1444..158ccafd2005e2b6839797360f344ca3bc1ad3e8 100644 (file)
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -593,10 +593,19 @@ HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *stmt)
 
         InitSymbolsInfo(&next_incl, info->keymap, rtrn->id, info->actions);
         next_incl.merge = next_incl.dflt.merge = MERGE_OVERRIDE;
-        if (stmt->modifier)
+        if (stmt->modifier) {
             next_incl.explicit_group = atoi(stmt->modifier) - 1;
-        else
+            if (next_incl.explicit_group >= XKB_NUM_GROUPS) {
+                log_err(info->keymap->ctx,
+                        "Cannot set explicit group to %d - must be between 1..%d; "
+                        "Ignoring group number\n",
+                        next_incl.explicit_group + 1, XKB_NUM_GROUPS);
+                next_incl.explicit_group = info->explicit_group;
+            }
+        }
+        else {
             next_incl.explicit_group = info->explicit_group;
+        }
 
         HandleSymbolsFile(&next_incl, rtrn, MERGE_OVERRIDE);
 

diff --git a/test/rulescomp.c b/test/rulescomp.c
index 5457d00e50c430faad25e4e431fc1b0137c8c462..3fb8a303306a4d75a8db5291a68941e4ed65fded 100644 (file)
--- a/test/rulescomp.c
+++ b/test/rulescomp.c
@@ -107,6 +107,9 @@ int main(int argc, char *argv[])
     assert(test_rmlvo(ctx, "evdev", "pc105", "us", "intl", ""));
     assert(test_rmlvo(ctx, "evdev", "evdev", "us", "intl", "grp:alts_toggle"));
 
+    /* 20 is not a legal group; make sure this is handled gracefully. */
+    assert(test_rmlvo(ctx, "evdev", "", "us:20", "", ""));
+
     assert(test_rmlvo(ctx, "", "", "", "", ""));
     assert(test_rmlvo(ctx, NULL, NULL, NULL, NULL, NULL));