CVE-2018-15854: NULL Pointer Dereference (CWE-476)
value: Unchecked NULL pointer usage in xkbcommon before 0.8.1 could be used by local attackers to crash (NULL pointer dereference) the xkbcommon parser by supplying a crafted keymap file, because geometry tokens were desupported incorrectly.
CWE description: The product dereferences a pointer that it expects to be valid but is NULL.
CVSS v3 base score: 5.5
Published: 2018-08-25T21:29Z
Repository: https://github.com/xkbcommon/libxkbcommon
Language: C
Files changed: src/xkbcomp/ast-build.c, src/xkbcomp/ast-build.h, src/xkbcomp/ast.h, src/xkbcomp/parser.y
Commit date: 2018-07-30 10:35:10 +0300
Commit message: xkbcomp: fix crashes in the parser when geometry tokens appear

In the XKB format, floats and various keywords can only be used in the
xkb_geometry section. xkbcommon removed support xkb_geometry, but still
parses it for backward compatibility. As part of ignoring it, the float
AST node and various keywords were removed, and instead NULL was
returned by their parsing actions. However, the rest of the code does
not handle NULLs, and so when they appear crashes usually ensue.

To fix this, restore the float AST node and the ignored keywords. None
of the evaluating code expects them, so nice error are displayed.

Caught with the afl fuzzer.

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

Diff: diff --git a/src/xkbcomp/ast-build.c b/src/xkbcomp/ast-build.c
index 58f97f93..9c6ccd44 100644
--- a/src/xkbcomp/ast-build.c
+++ b/src/xkbcomp/ast-build.c
@@ -103,10 +103,17 @@ ExprCreateInteger(int ival)
     EXPR_CREATE(ExprInteger, expr, EXPR_VALUE, EXPR_TYPE_INT);
     expr->integer.ival = ival;
     return expr;
 }
 
+ExprDef *
+ExprCreateFloat(void)
+{
+    EXPR_CREATE(ExprFloat, expr, EXPR_VALUE, EXPR_TYPE_FLOAT);
+    return expr;
+}
+
 ExprDef *
 ExprCreateBoolean(bool set)
 {
     EXPR_CREATE(ExprBoolean, expr, EXPR_VALUE, EXPR_TYPE_BOOLEAN);
     expr->boolean.set = set;
@@ -781,10 +788,11 @@ expr_op_type_to_string(enum expr_op_type type)
 
 static const char *expr_value_type_strings[_EXPR_TYPE_NUM_VALUES] = {
     [EXPR_TYPE_UNKNOWN] = "unknown",
     [EXPR_TYPE_BOOLEAN] = "boolean",
     [EXPR_TYPE_INT] = "int",
+    [EXPR_TYPE_FLOAT] = "float",
     [EXPR_TYPE_STRING] = "string",
     [EXPR_TYPE_ACTION] = "action",
     [EXPR_TYPE_KEYNAME] = "keyname",
     [EXPR_TYPE_SYMBOLS] = "symbols",
 };
diff --git a/src/xkbcomp/ast-build.h b/src/xkbcomp/ast-build.h
index b57e4cdc..6c76f381 100644
--- a/src/xkbcomp/ast-build.h
+++ b/src/xkbcomp/ast-build.h
@@ -34,10 +34,13 @@ ExprDef *
 ExprCreateString(xkb_atom_t str);
 
 ExprDef *
 ExprCreateInteger(int ival);
 
+ExprDef *
+ExprCreateFloat(void);
+
 ExprDef *
 ExprCreateBoolean(bool set);
 
 ExprDef *
 ExprCreateKeyName(xkb_atom_t key_name);
diff --git a/src/xkbcomp/ast.h b/src/xkbcomp/ast.h
index 97788840..49c5ada4 100644
--- a/src/xkbcomp/ast.h
+++ b/src/xkbcomp/ast.h
@@ -93,10 +93,11 @@ enum stmt_type {
 
 enum expr_value_type {
     EXPR_TYPE_UNKNOWN = 0,
     EXPR_TYPE_BOOLEAN,
     EXPR_TYPE_INT,
+    EXPR_TYPE_FLOAT,
     EXPR_TYPE_STRING,
     EXPR_TYPE_ACTION,
     EXPR_TYPE_KEYNAME,
     EXPR_TYPE_SYMBOLS,
 
@@ -184,10 +185,16 @@ typedef struct {
 typedef struct {
     ExprCommon expr;
     int ival;
 } ExprInteger;
 
+typedef struct {
+    ExprCommon expr;
+    /* We don't support floats, but we still represnt them in the AST, in
+     * order to provide proper error messages. */
+} ExprFloat;
+
 typedef struct {
     ExprCommon expr;
     xkb_atom_t key_name;
 } ExprKeyName;
 
diff --git a/src/xkbcomp/parser.y b/src/xkbcomp/parser.y
index ead20165..d15d24b3 100644
--- a/src/xkbcomp/parser.y
+++ b/src/xkbcomp/parser.y
@@ -589,17 +589,17 @@ Element         :       ACTION_TOK
                 |       MODIFIER_MAP
                         {$$ = xkb_atom_intern_literal(param->ctx, "modifier_map");}
                 |       INDICATOR
                         { $$ = xkb_atom_intern_literal(param->ctx, "indicator"); }
                 |       SHAPE
-                        { $$ = XKB_ATOM_NONE; }
+                        { $$ = xkb_atom_intern_literal(param->ctx, "shape"); }
                 |       ROW
-                        { $$ = XKB_ATOM_NONE; }
+                        { $$ = xkb_atom_intern_literal(param->ctx, "row"); }
                 |       SECTION
-                        { $$ = XKB_ATOM_NONE; }
+                        { $$ = xkb_atom_intern_literal(param->ctx, "section"); }
                 |       TEXT
-                        { $$ = XKB_ATOM_NONE; }
+                        { $$ = xkb_atom_intern_literal(param->ctx, "text"); }
                 ;
 
 OptMergeMode    :       MergeMode       { $$ = $1; }
                 |                       { $$ = MERGE_DEFAULT; }
                 ;
@@ -685,11 +685,11 @@ Lhs             :       FieldSpec
 Terminal        :       String
                         { $$ = ExprCreateString($1); }
                 |       Integer
                         { $$ = ExprCreateInteger($1); }
                 |       Float
-                        { $$ = NULL; }
+                        { $$ = ExprCreateFloat(/* Discard $1 */); }
                 |       KEYNAME
                         { $$ = ExprCreateKeyName($1); }
                 ;
 
 OptKeySymList   :       KeySymList      { $$ = $1; }

Vulnerable code:                         { $$ = XKB_ATOM_NONE; }
                        { $$ = XKB_ATOM_NONE; }
                        { $$ = XKB_ATOM_NONE; }
                        { $$ = XKB_ATOM_NONE; }
                        { $$ = NULL; }
Fixed code: ExprDef *
ExprCreateFloat(void)
{
    EXPR_CREATE(ExprFloat, expr, EXPR_VALUE, EXPR_TYPE_FLOAT);
    return expr;
}

    [EXPR_TYPE_FLOAT] = "float",
ExprDef *
ExprCreateFloat(void);

    EXPR_TYPE_FLOAT,
typedef struct {
    ExprCommon expr;
    /* We don't support floats, but we still represnt them in the AST, in
     * order to provide proper error messages. */
} ExprFloat;

                        { $$ = xkb_atom_intern_literal(param->ctx, "shape"); }
                        { $$ = xkb_atom_intern_literal(param->ctx, "row"); }
                        { $$ = xkb_atom_intern_literal(param->ctx, "section"); }
                        { $$ = xkb_atom_intern_literal(param->ctx, "text"); }
                        { $$ = ExprCreateFloat(/* Discard $1 */); }