Skip to content

Commit

Permalink
Prevent recursive includes of keymap components
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
wismill committed Nov 6, 2023
1 parent 4b58ff7 commit 00e3058
Show file tree
Hide file tree
Showing 15 changed files with 238 additions and 16 deletions.
10 changes: 10 additions & 0 deletions doc/message-registry.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand Down Expand Up @@ -327,6 +328,14 @@ as numbers and as identifiers `LevelN`, where `N` is in the range (1..8).
<dt>Summary</dt><dd>An entry is duplicated and will be ignored</dd>
</dl>

### XKB-386 – Recursive include {#XKB-386}

<dl>
<dt>Since</dt><dd>1.7.0</dd>
<dt>Type</dt><dd>Error</dd>
<dt>Summary</dt><dd>Included files form cycle</dd>
</dl>

### XKB-407 – Conflicting key type definitions {#XKB-407}

<dl>
Expand Down Expand Up @@ -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
Expand Down
5 changes: 5 additions & 0 deletions doc/message-registry.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/messages-codes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down
16 changes: 13 additions & 3 deletions src/xkbcomp/compat.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
16 changes: 14 additions & 2 deletions src/xkbcomp/include.c
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,20 @@ FindFileInXkbPath(struct xkb_context *ctx, const char *name,
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)
Expand Down Expand Up @@ -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;
}
6 changes: 6 additions & 0 deletions src/xkbcomp/include.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
16 changes: 12 additions & 4 deletions src/xkbcomp/keycodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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."
Expand Down Expand Up @@ -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;

Expand All @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
17 changes: 13 additions & 4 deletions src/xkbcomp/symbols.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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;

Expand All @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
15 changes: 12 additions & 3 deletions src/xkbcomp/types.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ typedef struct {
typedef struct {
char *name;
int errorCount;
unsigned int include_depth;

darray(KeyTypeInfo) types;
struct xkb_mod_set mods;
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand All @@ -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);

Expand Down Expand Up @@ -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)
Expand Down
Loading

0 comments on commit 00e3058

Please sign in to comment.