From e209fe31dff4c5eaad0c61f48ef4f8ca7a90e26d Mon Sep 17 00:00:00 2001
From: Daniel Stone <daniel@fooishbar.org>
Date: Mon, 20 Feb 2012 17:33:39 +0000
Subject: [PATCH] Perform bounds checking in ExprResolveGroup

Every caller did the exact same check on the group bounds after calling
ExprResolveGroup, so might as well do it inside.

Signed-off-by: Daniel Stone <daniel@fooishbar.org>
---
 src/xkbcomp/action.c  |  7 -------
 src/xkbcomp/expr.c    | 14 ++++++++++++--
 src/xkbcomp/symbols.c | 47 +++--------------------------------------------
 3 files changed, 15 insertions(+), 53 deletions(-)

diff --git a/src/xkbcomp/action.c b/src/xkbcomp/action.c
index 95afcde..2debae7 100644
--- a/src/xkbcomp/action.c
+++ b/src/xkbcomp/action.c
@@ -482,13 +482,6 @@ CheckGroupField(unsigned action,
 
     if (!ExprResolveGroup(spec, &rtrn))
         return ReportMismatch(action, F_Group, "integer (range 1..8)");
-    if ((rtrn.ival < 1) || (rtrn.ival > XkbNumKbdGroups))
-    {
-        ERROR("Illegal group %d (must be in the range 1..%d)\n", rtrn.ival,
-               XkbNumKbdGroups);
-        ACTION("Action %s definition ignored\n", XkbcActionTypeText(action));
-        return False;
-    }
     if (value->op == OpNegate)
         *grp_rtrn = -rtrn.ival;
     else if (value->op == OpUnaryPlus)
diff --git a/src/xkbcomp/expr.c b/src/xkbcomp/expr.c
index d175123..c371e98 100644
--- a/src/xkbcomp/expr.c
+++ b/src/xkbcomp/expr.c
@@ -643,6 +643,7 @@ int
 ExprResolveGroup(ExprDef * expr,
                  ExprResult * val_rtrn)
 {
+    int ret;
     static LookupEntry group_names[] = {
         { "group1", 1 },
         { "group2", 2 },
@@ -655,8 +656,17 @@ ExprResolveGroup(ExprDef * expr,
         { NULL, 0 }
     };
 
-    return ExprResolveIntegerLookup(expr, val_rtrn, SimpleLookup,
-                                    group_names);
+    ret = ExprResolveIntegerLookup(expr, val_rtrn, SimpleLookup, group_names);
+    if (ret == False)
+        return ret;
+
+    if (val_rtrn->uval == 0 || val_rtrn->uval > XkbNumKbdGroups) {
+        ERROR("Group index %d is out of range (1..%d)\n",
+              val_rtrn->uval, XkbNumKbdGroups);
+        return False;
+    }
+
+    return True;
 }
 
 int
diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c
index 6011c26..f646e76 100644
--- a/src/xkbcomp/symbols.c
+++ b/src/xkbcomp/symbols.c
@@ -904,13 +904,6 @@ GetGroupIndex(KeyInfo * key,
         ACTION("Definition with non-integer array index ignored\n");
         return False;
     }
-    if ((tmp.uval < 1) || (tmp.uval > XkbNumKbdGroups))
-    {
-        ERROR("Group index for %s of key %s is out of range (1..%d)\n",
-               name, longText(key->name), XkbNumKbdGroups + 1);
-        ACTION("Ignoring %s for group %d\n", name, tmp.uval);
-        return False;
-    }
     *ndx_rtrn = tmp.uval - 1;
     return True;
 }
@@ -1131,15 +1124,6 @@ SetSymbolsField(KeyInfo * key,
             free(tmp.str);
             return False;
         }
-        else if ((ndx.uval < 1) || (ndx.uval > XkbNumKbdGroups))
-        {
-            ERROR
-                ("Group index for type of key %s is out of range (1..%d)\n",
-                 longText(key->name), XkbNumKbdGroups + 1);
-            ACTION("Ignoring type for group %d\n", ndx.uval);
-            free(tmp.str);
-            return False;
-        }
         else
         {
             key->types[ndx.uval - 1] = xkb_intern_atom(tmp.str);
@@ -1337,13 +1321,6 @@ SetSymbolsField(KeyInfo * key,
             ACTION("Definition with non-integer group ignored\n");
             return False;
         }
-        if ((tmp.uval < 1) || (tmp.uval > XkbNumKbdGroups))
-        {
-            ERROR("Out-of-range (1..%d) group for redirect of key %s\n",
-                   XkbNumKbdGroups, longText(key->name));
-            ERROR("Ignoring illegal group %d\n", tmp.uval);
-            return False;
-        }
         key->groupInfo =
             XkbSetGroupInfo(0, XkbRedirectIntoRange, tmp.uval - 1);
         key->defs.defined |= _Key_GroupInfo;
@@ -1374,14 +1351,6 @@ SetGroupName(SymbolsInfo * info, ExprDef * arrayNdx, ExprDef * value)
         ACTION("Definition with non-integer array index ignored\n");
         return False;
     }
-    if ((tmp.uval < 1) || (tmp.uval > XkbNumKbdGroups))
-    {
-        ERROR
-            ("Attempt to specify name for illegal group (must be 1..%d)\n",
-             XkbNumKbdGroups + 1);
-        ACTION("Name for group %d ignored\n", tmp.uval);
-        return False;
-    }
     if (!ExprResolveString(value, &name))
     {
         ERROR("Group name must be a string\n");
@@ -1462,19 +1431,9 @@ HandleSymbolsVar(VarDef * stmt, struct xkb_desc * xkb, SymbolsInfo * info)
             ret = False;
         }
         else {
-            if ((tmp.uval < 1) || (tmp.uval > XkbNumKbdGroups))
-            {
-                ERROR
-                    ("Out-of-range (1..%d) group for global groupsRedirect\n",
-                     XkbNumKbdGroups);
-                ACTION("Ignoring illegal group %d\n", tmp.uval);
-                ret = False;
-            }
-            else {
-                info->groupInfo = XkbSetGroupInfo(0, XkbRedirectIntoRange,
-                                                  tmp.uval);
-                ret = True;
-            }
+            info->groupInfo = XkbSetGroupInfo(0, XkbRedirectIntoRange,
+                                              tmp.uval);
+            ret = True;
         }
     }
     else if ((elem.str == NULL) && (uStrCaseCmp(field.str, "allownone") == 0))
-- 
2.7.4