From 00e3058e7b027c3d698b24415fd2ac105cd72246 Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Mon, 6 Nov 2023 21:53:51 +0100 Subject: [PATCH] Prevent recursive includes of keymap components - Add check for recursive includes of keymap components. It relies on limiting the include depth. The threshold is currently to 15, which seems reasonable with plenty of margin for keymaps in the wild. - Add corresponding new log message `recursive-include`. - Add tests for recursive includes. --- doc/message-registry.md | 10 +++++ doc/message-registry.yaml | 5 +++ src/messages-codes.h | 2 + src/xkbcomp/compat.c | 16 +++++-- src/xkbcomp/include.c | 16 ++++++- src/xkbcomp/include.h | 6 +++ src/xkbcomp/keycodes.c | 16 +++++-- src/xkbcomp/symbols.c | 17 ++++++-- src/xkbcomp/types.c | 15 +++++-- test/buffercomp.c | 84 ++++++++++++++++++++++++++++++++++++ test/data/compat/recursive | 12 ++++++ test/data/keycodes/recursive | 15 +++++++ test/data/symbols/recursive | 12 ++++++ test/data/types/recursive | 27 ++++++++++++ tools/messages.c | 1 + 15 files changed, 238 insertions(+), 16 deletions(-) create mode 100644 test/data/compat/recursive create mode 100644 test/data/keycodes/recursive create mode 100644 test/data/symbols/recursive create mode 100644 test/data/types/recursive diff --git a/doc/message-registry.md b/doc/message-registry.md index b5358f0..c3e3ec0 100644 --- a/doc/message-registry.md +++ b/doc/message-registry.md @@ -39,6 +39,7 @@ There are currently 54 entries. | [XKB-338] | `included-file-not-found` | Could not find a file used in an include statement | Error | | [XKB-345] | `unknown-operator` | Use of an operator that is unknown and thus unsupported | Error | | [XKB-378] | `duplicate-entry` | An entry is duplicated and will be ignored | Warning | +| [XKB-386] | `recursive-include` | Included files form cycle | Error | | [XKB-407] | `conflicting-key-type-definitions` | Conflicting definitions of a key type | Warning | | [XKB-428] | `wrong-scope` | A statement is in a wrong scope and should be moved | Error | | [XKB-433] | `missing-default-section` | Missing default section in included file | Warning | @@ -327,6 +328,14 @@ as numbers and as identifiers `LevelN`, where `N` is in the range (1..8).
Summary
An entry is duplicated and will be ignored
+### XKB-386 – Recursive include {#XKB-386} + +
+
Since
1.7.0
+
Type
Error
+
Summary
Included files form cycle
+
+ ### XKB-407 – Conflicting key type definitions {#XKB-407}
@@ -657,6 +666,7 @@ The modifiers used in `map` or `preserve` entries should be declared using the e [XKB-338]: @ref XKB-338 [XKB-345]: @ref XKB-345 [XKB-378]: @ref XKB-378 +[XKB-386]: @ref XKB-386 [XKB-407]: @ref XKB-407 [XKB-428]: @ref XKB-428 [XKB-433]: @ref XKB-433 diff --git a/doc/message-registry.yaml b/doc/message-registry.yaml index 0b7db81..ff1fece 100644 --- a/doc/message-registry.yaml +++ b/doc/message-registry.yaml @@ -180,6 +180,11 @@ added: ALWAYS type: warning description: "An entry is duplicated and will be ignored" +- id: "recursive-include" + code: 386 + added: 1.7.0 + type: error + description: "Included files form cycle" - id: "conflicting-key-type-definitions" code: 407 added: ALWAYS diff --git a/src/messages-codes.h b/src/messages-codes.h index 3206831..b90677f 100644 --- a/src/messages-codes.h +++ b/src/messages-codes.h @@ -66,6 +66,8 @@ enum xkb_message_code { XKB_ERROR_UNKNOWN_OPERATOR = 345, /** An entry is duplicated and will be ignored */ XKB_WARNING_DUPLICATE_ENTRY = 378, + /** Included files form cycle */ + XKB_ERROR_RECURSIVE_INCLUDE = 386, /** Conflicting definitions of a key type */ XKB_WARNING_CONFLICTING_KEY_TYPE_DEFINITIONS = 407, /** A statement is in a wrong scope and should be moved */ diff --git a/src/xkbcomp/compat.c b/src/xkbcomp/compat.c index 9b6f9de..1df9e03 100644 --- a/src/xkbcomp/compat.c +++ b/src/xkbcomp/compat.c @@ -86,6 +86,7 @@ typedef struct { typedef struct { char *name; int errorCount; + unsigned int include_depth; SymInterpInfo default_interp; darray(SymInterpInfo) interps; LedInfo default_led; @@ -148,10 +149,12 @@ ReportLedNotArray(CompatInfo *info, LedInfo *ledi, const char *field) static void InitCompatInfo(CompatInfo *info, struct xkb_context *ctx, + unsigned int include_depth, ActionsInfo *actions, const struct xkb_mod_set *mods) { memset(info, 0, sizeof(*info)); info->ctx = ctx; + info->include_depth = include_depth; info->actions = actions; info->mods = *mods; info->default_interp.merge = MERGE_OVERRIDE; @@ -430,7 +433,13 @@ HandleIncludeCompatMap(CompatInfo *info, IncludeStmt *include) { CompatInfo included; - InitCompatInfo(&included, info->ctx, info->actions, &info->mods); + if (ExceedsIncludeMaxDepth(info->ctx, info->include_depth)) { + info->errorCount += 10; + return false; + } + + InitCompatInfo(&included, info->ctx, 0 /* unused */, + info->actions, &info->mods); included.name = include->stmt; include->stmt = NULL; @@ -445,7 +454,8 @@ HandleIncludeCompatMap(CompatInfo *info, IncludeStmt *include) return false; } - InitCompatInfo(&next_incl, info->ctx, info->actions, &included.mods); + InitCompatInfo(&next_incl, info->ctx, info->include_depth + 1, + info->actions, &included.mods); next_incl.default_interp = info->default_interp; next_incl.default_interp.merge = stmt->merge; next_incl.default_led = info->default_led; @@ -914,7 +924,7 @@ CompileCompatMap(XkbFile *file, struct xkb_keymap *keymap, if (!actions) return false; - InitCompatInfo(&info, keymap->ctx, actions, &keymap->mods); + InitCompatInfo(&info, keymap->ctx, 0, actions, &keymap->mods); info.default_interp.merge = merge; info.default_led.merge = merge; diff --git a/src/xkbcomp/include.c b/src/xkbcomp/include.c index 1fa2153..31df8d2 100644 --- a/src/xkbcomp/include.c +++ b/src/xkbcomp/include.c @@ -286,6 +286,20 @@ out: return file; } +bool +ExceedsIncludeMaxDepth(struct xkb_context *ctx, unsigned int include_depth) +{ + if (include_depth >= INCLUDE_MAX_DEPTH) { + log_err(ctx, + XKB_ERROR_RECURSIVE_INCLUDE, + "Exceeded include depth threshold (%d)", + INCLUDE_MAX_DEPTH); + return true; + } else { + return false; + } +} + XkbFile * ProcessIncludeFile(struct xkb_context *ctx, IncludeStmt *stmt, enum xkb_file_type file_type) @@ -334,7 +348,5 @@ ProcessIncludeFile(struct xkb_context *ctx, IncludeStmt *stmt, stmt->file); } - /* FIXME: we have to check recursive includes here (or somewhere) */ - return xkb_file; } diff --git a/src/xkbcomp/include.h b/src/xkbcomp/include.h index 8f360e3..5a5d331 100644 --- a/src/xkbcomp/include.h +++ b/src/xkbcomp/include.h @@ -27,6 +27,9 @@ #ifndef XKBCOMP_INCLUDE_H #define XKBCOMP_INCLUDE_H +/* Reasonable threshold, with plenty of margin for keymaps in the wild */ +#define INCLUDE_MAX_DEPTH 15 + bool ParseIncludeMap(char **str_inout, char **file_rtrn, char **map_rtrn, char *nextop_rtrn, char **extra_data); @@ -36,6 +39,9 @@ FindFileInXkbPath(struct xkb_context *ctx, const char *name, enum xkb_file_type type, char **pathRtrn, unsigned int *offset); +bool +ExceedsIncludeMaxDepth(struct xkb_context *ctx, unsigned int include_depth); + XkbFile * ProcessIncludeFile(struct xkb_context *ctx, IncludeStmt *stmt, enum xkb_file_type file_type); diff --git a/src/xkbcomp/keycodes.c b/src/xkbcomp/keycodes.c index 91471ea..700cd1c 100644 --- a/src/xkbcomp/keycodes.c +++ b/src/xkbcomp/keycodes.c @@ -47,6 +47,7 @@ typedef struct { typedef struct { char *name; int errorCount; + unsigned int include_depth; xkb_keycode_t min_key_code; xkb_keycode_t max_key_code; @@ -155,10 +156,12 @@ ClearKeyNamesInfo(KeyNamesInfo *info) } static void -InitKeyNamesInfo(KeyNamesInfo *info, struct xkb_context *ctx) +InitKeyNamesInfo(KeyNamesInfo *info, struct xkb_context *ctx, + unsigned int include_depth) { memset(info, 0, sizeof(*info)); info->ctx = ctx; + info->include_depth = include_depth; info->min_key_code = XKB_KEYCODE_INVALID; #if XKB_KEYCODE_INVALID < XKB_KEYCODE_MAX #error "Hey, you can't be changing stuff like that." @@ -339,7 +342,12 @@ HandleIncludeKeycodes(KeyNamesInfo *info, IncludeStmt *include) { KeyNamesInfo included; - InitKeyNamesInfo(&included, info->ctx); + if (ExceedsIncludeMaxDepth(info->ctx, info->include_depth)) { + info->errorCount += 10; + return false; + } + + InitKeyNamesInfo(&included, info->ctx, 0 /* unused */); included.name = include->stmt; include->stmt = NULL; @@ -354,7 +362,7 @@ HandleIncludeKeycodes(KeyNamesInfo *info, IncludeStmt *include) return false; } - InitKeyNamesInfo(&next_incl, info->ctx); + InitKeyNamesInfo(&next_incl, info->ctx, info->include_depth + 1); HandleKeycodesFile(&next_incl, file, MERGE_OVERRIDE); @@ -662,7 +670,7 @@ CompileKeycodes(XkbFile *file, struct xkb_keymap *keymap, { KeyNamesInfo info; - InitKeyNamesInfo(&info, keymap->ctx); + InitKeyNamesInfo(&info, keymap->ctx, 0); HandleKeycodesFile(&info, file, merge); if (info.errorCount != 0) diff --git a/src/xkbcomp/symbols.c b/src/xkbcomp/symbols.c index 20eebeb..0c4fbeb 100644 --- a/src/xkbcomp/symbols.c +++ b/src/xkbcomp/symbols.c @@ -175,6 +175,7 @@ typedef struct { typedef struct { char *name; /* e.g. pc+us+inet(evdev) */ int errorCount; + unsigned int include_depth; enum merge_mode merge; xkb_layout_index_t explicit_group; darray(KeyInfo) keys; @@ -191,10 +192,12 @@ typedef struct { static void InitSymbolsInfo(SymbolsInfo *info, const struct xkb_keymap *keymap, + unsigned int include_depth, ActionsInfo *actions, const struct xkb_mod_set *mods) { memset(info, 0, sizeof(*info)); info->ctx = keymap->ctx; + info->include_depth = include_depth; info->keymap = keymap; info->merge = MERGE_OVERRIDE; InitKeyInfo(keymap->ctx, &info->default_key); @@ -569,7 +572,13 @@ HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *include) { SymbolsInfo included; - InitSymbolsInfo(&included, info->keymap, info->actions, &info->mods); + if (ExceedsIncludeMaxDepth(info->ctx, info->include_depth)) { + info->errorCount += 10; + return false; + } + + InitSymbolsInfo(&included, info->keymap, 0 /* unused */, + info->actions, &info->mods); included.name = include->stmt; include->stmt = NULL; @@ -584,8 +593,8 @@ HandleIncludeSymbols(SymbolsInfo *info, IncludeStmt *include) return false; } - InitSymbolsInfo(&next_incl, info->keymap, info->actions, - &included.mods); + InitSymbolsInfo(&next_incl, info->keymap, info->include_depth + 1, + info->actions, &included.mods); if (stmt->modifier) { next_incl.explicit_group = atoi(stmt->modifier) - 1; if (next_incl.explicit_group >= XKB_MAX_GROUPS) { @@ -1638,7 +1647,7 @@ CompileSymbols(XkbFile *file, struct xkb_keymap *keymap, if (!actions) return false; - InitSymbolsInfo(&info, keymap, actions, &keymap->mods); + InitSymbolsInfo(&info, keymap, 0, actions, &keymap->mods); info.default_key.merge = merge; HandleSymbolsFile(&info, file, merge); diff --git a/src/xkbcomp/types.c b/src/xkbcomp/types.c index 183aa50..f22c9ef 100644 --- a/src/xkbcomp/types.c +++ b/src/xkbcomp/types.c @@ -53,6 +53,7 @@ typedef struct { typedef struct { char *name; int errorCount; + unsigned int include_depth; darray(KeyTypeInfo) types; struct xkb_mod_set mods; @@ -100,10 +101,12 @@ ReportTypeBadType(KeyTypesInfo *info, xkb_message_code_t code, static void InitKeyTypesInfo(KeyTypesInfo *info, struct xkb_context *ctx, + unsigned int include_depth, const struct xkb_mod_set *mods) { memset(info, 0, sizeof(*info)); info->ctx = ctx; + info->include_depth = include_depth; info->mods = *mods; } @@ -219,7 +222,12 @@ HandleIncludeKeyTypes(KeyTypesInfo *info, IncludeStmt *include) { KeyTypesInfo included; - InitKeyTypesInfo(&included, info->ctx, &info->mods); + if (ExceedsIncludeMaxDepth(info->ctx, info->include_depth)) { + info->errorCount += 10; + return false; + } + + InitKeyTypesInfo(&included, info->ctx, 0 /* unused */, &info->mods); included.name = include->stmt; include->stmt = NULL; @@ -234,7 +242,8 @@ HandleIncludeKeyTypes(KeyTypesInfo *info, IncludeStmt *include) return false; } - InitKeyTypesInfo(&next_incl, info->ctx, &included.mods); + InitKeyTypesInfo(&next_incl, info->ctx, info->include_depth + 1, + &included.mods); HandleKeyTypesFile(&next_incl, file, stmt->merge); @@ -754,7 +763,7 @@ CompileKeyTypes(XkbFile *file, struct xkb_keymap *keymap, { KeyTypesInfo info; - InitKeyTypesInfo(&info, keymap->ctx, &keymap->mods); + InitKeyTypesInfo(&info, keymap->ctx, 0, &keymap->mods); HandleKeyTypesFile(&info, file, merge); if (info.errorCount != 0) diff --git a/test/buffercomp.c b/test/buffercomp.c index 091a876..f111763 100644 --- a/test/buffercomp.c +++ b/test/buffercomp.c @@ -84,6 +84,88 @@ test_encodings(struct xkb_context *ctx) return true; } +static void +test_recursive(void) +{ + struct xkb_context *ctx = test_get_context(0); + struct xkb_keymap *keymap; + + assert(ctx); + + const char* const keymaps[] = { + /* Recursive keycodes */ + "Keycodes: recursive", + "xkb_keymap {" + " xkb_keycodes { include \"evdev+recursive\" };" + " xkb_types { include \"complete\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"pc\" };" + "};", + "Keycodes: recursive(bar)", + "xkb_keymap {" + " xkb_keycodes { include \"evdev+recursive(bar)\" };" + " xkb_types { include \"complete\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"pc\" };" + "};", + /* Recursive key types */ + "Key types: recursive", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"recursive\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"pc\" };" + "};", + "Key types: recursive(bar)", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"recursive(bar)\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"pc\" };" + "};", + /* Recursive compat */ + "Compat: recursive", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"recursive\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"pc\" };" + "};", + "Compat: recursive(bar)", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"complete\" };" + " xkb_compat { include \"recursive(bar)\" };" + " xkb_symbols { include \"pc\" };" + "};", + /* Recursive symbols */ + "Symbols: recursive", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"complete\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"recursive\" };" + "};", + "Symbols: recursive(bar)", + "xkb_keymap {" + " xkb_keycodes { include \"evdev\" };" + " xkb_types { include \"complete\" };" + " xkb_compat { include \"complete\" };" + " xkb_symbols { include \"recursive(bar)\" };" + // "};" + }; + + int len = sizeof(keymaps) / sizeof(keymaps[0]); + + for (int k = 0; k < len; k++) { + fprintf(stderr, "*** Recursive test: %s ***\n", keymaps[k++]); + keymap = test_compile_buffer(ctx, keymaps[k], strlen(keymaps[k])); + assert(!keymap); + } + + xkb_context_unref(ctx); +} + int main(int argc, char *argv[]) { @@ -147,5 +229,7 @@ main(int argc, char *argv[]) xkb_context_unref(ctx); + test_recursive(); + return 0; } diff --git a/test/data/compat/recursive b/test/data/compat/recursive new file mode 100644 index 0000000..3273ead --- /dev/null +++ b/test/data/compat/recursive @@ -0,0 +1,12 @@ +default +xkb_compatibility "foo" { + include "recursive" +}; + +xkb_compatibility "bar" { + include "recursive(baz)" +}; + +xkb_compatibility "baz" { + include "recursive(bar)" +}; diff --git a/test/data/keycodes/recursive b/test/data/keycodes/recursive new file mode 100644 index 0000000..6711a60 --- /dev/null +++ b/test/data/keycodes/recursive @@ -0,0 +1,15 @@ +default +xkb_keycodes "foo" { + include "recursive" + = 0x100000; +}; + +xkb_keycodes "bar" { + include "recursive(baz)" + = 0x100001; +}; + +xkb_keycodes "baz" { + include "recursive(bar)" + = 0x100002; +}; diff --git a/test/data/symbols/recursive b/test/data/symbols/recursive new file mode 100644 index 0000000..2a9ad63 --- /dev/null +++ b/test/data/symbols/recursive @@ -0,0 +1,12 @@ +default +xkb_symbols "foo" { + include "recursive" +}; + +xkb_symbols "bar" { + include "recursive(baz)" +}; + +xkb_symbols "baz" { + include "recursive(bar)" +}; diff --git a/test/data/types/recursive b/test/data/types/recursive new file mode 100644 index 0000000..4b75a34 --- /dev/null +++ b/test/data/types/recursive @@ -0,0 +1,27 @@ +default +xkb_types "foo" { + type "FOO" { + modifiers = None; + map[None] = Level1; + level_name[Level1]= "Any"; + }; + include "recursive" +}; + +xkb_types "bar" { + type "BAR" { + modifiers = None; + map[None] = Level1; + level_name[Level1]= "Any"; + }; + include "recursive(baz)" +}; + +xkb_types "baz" { + type "BAZ" { + modifiers = None; + map[None] = Level1; + level_name[Level1]= "Any"; + }; + include "recursive(bar)" +}; diff --git a/tools/messages.c b/tools/messages.c index a0ffd95..ad819a5 100644 --- a/tools/messages.c +++ b/tools/messages.c @@ -63,6 +63,7 @@ static const struct xkb_message_entry xkb_messages[] = { {XKB_ERROR_INCLUDED_FILE_NOT_FOUND, "Included file not found"}, {XKB_ERROR_UNKNOWN_OPERATOR, "Unknown operator"}, {XKB_WARNING_DUPLICATE_ENTRY, "Duplicate entry"}, + {XKB_ERROR_RECURSIVE_INCLUDE, "Recursive include"}, {XKB_WARNING_CONFLICTING_KEY_TYPE_DEFINITIONS, "Conflicting key type definitions"}, {XKB_ERROR_WRONG_SCOPE, "Wrong scope"}, {XKB_WARNING_MISSING_DEFAULT_SECTION, "Missing default section"},