Disallow producing NULL with escape sequences & prevent overflow

doc/message-registry.md

@@ -6,7 +6,7 @@ NOTE: This file has been generated automatically by “update-message-registry.p
 -->
 
 This page lists the warnings and errors generated by xkbcommon.
-There are currently 52 entries.
+There are currently 53 entries.
 
 @todo The documentation of the log messages is a work in progress.
 
@@ -24,6 +24,7 @@ There are currently 52 entries.
 | [XKB-150] | `wrong-statement-type` | The type of the statement is not allowed in the context | Error |
 | [XKB-172] | `unsupported-geometry-section` | Geometry sections are not supported | Warning |
 | [XKB-183] | `cannot-infer-key-type` | Warn if no key type can be inferred | Warning |
+| [XKB-193] | `invalid-escape-sequence` | Invalid escape sequence in a string | Warning |
 | [XKB-195] | `illegal-key-type-preserve-result` | The result of a key type “preserve” entry must be a subset of its input modifiers. | Warning |
 | [XKB-203] | `invalid-include-statement` | Syntax error in the include statement | Error |
 | [XKB-206] | `invalid-modmap-entry` | A modmap entry is invalid | Error |
@@ -198,6 +199,14 @@ key <AB08> {[ comma, semicolon, periodcentered, multiply ]};
   <dt>Summary</dt><dd>Warn if no key type can be inferred</dd>
 </dl>
 
+### XKB-193 – Invalid escape sequence {#XKB-193}
+
+<dl>
+  <dt>Since</dt><dd>1.0.0</dd>
+  <dt>Type</dt><dd>Warning</dd>
+  <dt>Summary</dt><dd>Invalid escape sequence in a string</dd>
+</dl>
+
 ### XKB-195 – Illegal key type preserve result {#XKB-195}
 
 <dl>
@@ -624,6 +633,7 @@ The modifiers used in `map` or `preserve` entries should be declared using the e
 [XKB-150]: @ref XKB-150
 [XKB-172]: @ref XKB-172
 [XKB-183]: @ref XKB-183
+[XKB-193]: @ref XKB-193
 [XKB-195]: @ref XKB-195
 [XKB-203]: @ref XKB-203
 [XKB-206]: @ref XKB-206

doc/message-registry.yaml

@@ -100,6 +100,11 @@
   added: ALWAYS
   type: warning
   description: "Warn if no key type can be inferred"
+- id: "invalid-escape-sequence"
+  code: 193
+  added: ALWAYS
+  type: warning
+  description: "Invalid escape sequence in a string"
 - id: "illegal-key-type-preserve-result"
   code: 195
   added: ALWAYS

src/compose/parser.c

@@ -171,23 +171,38 @@ lex(struct scanner *s, union lvalue *val)
         while (!scanner_eof(s) && !scanner_eol(s) && scanner_peek(s) != '\"') {
             if (scanner_chr(s, '\\')) {
                 uint8_t o;
+                size_t start_pos = s->pos;
                 if (scanner_chr(s, '\\')) {
                     scanner_buf_append(s, '\\');
                 }
                 else if (scanner_chr(s, '"')) {
                     scanner_buf_append(s, '"');
                 }
                 else if (scanner_chr(s, 'x') || scanner_chr(s, 'X')) {
-                    if (scanner_hex(s, &o))
+                    if (scanner_hex(s, &o) && is_valid_char((char) o)) {
                         scanner_buf_append(s, (char) o);
-                    else
-                        scanner_warn(s, "illegal hexadecimal escape sequence in string literal");
+                    } else {
+                        scanner_warn_with_code(s,
+                            XKB_WARNING_INVALID_ESCAPE_SEQUENCE,
+                            "illegal hexadecimal escape sequence (%.*s) in string literal",
+                            (int) (s->pos - start_pos + 1), &s->s[start_pos - 1]);
+                    }
                 }
-                else if (scanner_oct(s, &o)) {
+                else if (scanner_oct(s, &o) && is_valid_char((char) o)) {
                     scanner_buf_append(s, (char) o);
                 }
+                else if (s->pos > start_pos) {
+                    scanner_warn_with_code(s,
+                        XKB_WARNING_INVALID_ESCAPE_SEQUENCE,
+                        "illegal octal escape sequence (%.*s) in string literal",
+                        (int) (s->pos - start_pos + 1), &s->s[start_pos - 1]);
+                    /* Ignore. */
+                }
                 else {
-                    scanner_warn(s, "unknown escape sequence (%c) in string literal", scanner_peek(s));
+                    scanner_warn_with_code(s,
+                        XKB_WARNING_UNKNOWN_CHAR_ESCAPE_SEQUENCE,
+                        "unknown escape sequence (\\%c) in string literal",
+                        scanner_peek(s));
                     /* Ignore. */
                 }
             } else {

src/messages-codes.h

@@ -36,6 +36,8 @@ enum xkb_message_code {
     XKB_WARNING_UNSUPPORTED_GEOMETRY_SECTION = 172,
     /** Warn if no key type can be inferred */
     XKB_WARNING_CANNOT_INFER_KEY_TYPE = 183,
+    /** Invalid escape sequence in a string */
+    XKB_WARNING_INVALID_ESCAPE_SEQUENCE = 193,
     /** The result of a key type “preserve” entry must be a subset of its input modifiers. */
     XKB_WARNING_ILLEGAL_KEY_TYPE_PRESERVE_RESULT = 195,
     /** Syntax error in the include statement */

src/scanner-utils.h

@@ -188,7 +188,14 @@ scanner_oct(struct scanner *s, uint8_t *out)
 {
     int i;
     for (i = 0, *out = 0; scanner_peek(s) >= '0' && scanner_peek(s) <= '7' && i < 3; i++)
-        *out = *out * 8 + scanner_next(s) - '0';
+        /* Test overflow */
+        if (*out < 040) {
+            *out = *out * 8 + scanner_next(s) - '0';
+        } else {
+            /* Consume valid digit, but mark result as invalid */
+            scanner_next(s);
+            return false;
+        }
     return i > 0;
 }
 

src/utils.h

@@ -65,6 +65,15 @@
 #define STRINGIFY(x) #x
 #define STRINGIFY2(x) STRINGIFY(x)
 
+/* Check if a character is valid in a string literal */
+static inline bool
+is_valid_char(char c)
+{
+    /* Currently we only check for NULL character, but this could be extended
+     * in the future to further ASCII control characters. */
+    return c != 0;
+}
+
 char
 to_lower(char c);
 

src/xkbcomp/scanner.c

@@ -90,6 +90,7 @@ _xkbcommon_lex(YYSTYPE *yylval, struct scanner *s)
         while (!scanner_eof(s) && !scanner_eol(s) && scanner_peek(s) != '\"') {
             if (scanner_chr(s, '\\')) {
                 uint8_t o;
+                size_t start_pos = s->pos;
                 if      (scanner_chr(s, '\\')) scanner_buf_append(s, '\\');
                 else if (scanner_chr(s, 'n'))  scanner_buf_append(s, '\n');
                 else if (scanner_chr(s, 't'))  scanner_buf_append(s, '\t');
@@ -98,12 +99,19 @@ _xkbcommon_lex(YYSTYPE *yylval, struct scanner *s)
                 else if (scanner_chr(s, 'f'))  scanner_buf_append(s, '\f');
                 else if (scanner_chr(s, 'v'))  scanner_buf_append(s, '\v');
                 else if (scanner_chr(s, 'e'))  scanner_buf_append(s, '\033');
-                else if (scanner_oct(s, &o))   scanner_buf_append(s, (char) o);
+                else if (scanner_oct(s, &o) && is_valid_char((char) o))
+                    scanner_buf_append(s, (char) o);
+                else if (s->pos > start_pos)
+                    scanner_warn_with_code(s,
+                        XKB_WARNING_INVALID_ESCAPE_SEQUENCE,
+                        "invalid octal escape sequence (%.*s) in string literal",
+                        (int) (s->pos - start_pos + 1), &s->s[start_pos - 1]);
+                    /* Ignore. */
                 else {
-                    // TODO: display actual sequence! See: scanner_peek(s).
-                    //       require escaping any potential control character
-                    scanner_warn_with_code(s, XKB_WARNING_UNKNOWN_CHAR_ESCAPE_SEQUENCE,
-                                           "unknown escape sequence in string literal");
+                    scanner_warn_with_code(s,
+                        XKB_WARNING_UNKNOWN_CHAR_ESCAPE_SEQUENCE,
+                        "unknown escape sequence (\\%c) in string literal",
+                        scanner_peek(s));
                     /* Ignore. */
                 }
             } else {

test/compose.c

@@ -684,6 +684,21 @@ test_traverse(struct xkb_context *ctx)
     xkb_compose_table_unref(table);
 }
 
+static void
+test_escape_sequences(struct xkb_context *ctx)
+{
+    /* The following escape sequences should be ignored:
+     * • \401 overflows
+     * • \0 and \x0 produce NULL
+     */
+    const char *table_string = "<o> <e> : \"\\401f\\x0o\\0o\" X\n";
+
+    assert(test_compose_seq_buffer(ctx, table_string,
+        XKB_KEY_o, XKB_COMPOSE_FEED_ACCEPTED, XKB_COMPOSE_COMPOSING,  "",     XKB_KEY_NoSymbol,
+        XKB_KEY_e, XKB_COMPOSE_FEED_ACCEPTED, XKB_COMPOSE_COMPOSED,   "foo",  XKB_KEY_X,
+        XKB_KEY_NoSymbol));
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -717,6 +732,7 @@ main(int argc, char *argv[])
     test_include(ctx);
     test_override(ctx);
     test_traverse(ctx);
+    test_escape_sequences(ctx);
 
     xkb_context_unref(ctx);
     return 0;

test/data/keymaps/invalid-escape-sequence.xkb

@@ -0,0 +1,11 @@
+xkb_keymap {
+    // The following include statement has an octal escape sequence that
+    // must be ignored. Else it would insert a NULL character and thus
+    // truncates the string to "evde", while we expect "evdev+aliases(qwerty)".
+    xkb_keycodes  { include "evde\0v+aliases(qwerty)" };
+    // The following include statement has two octal escape sequences that
+    // should be ignored, else they would overflow.
+    xkb_types     { include "com\401ple\777te" };
+    xkb_compat    { include "complete" };
+    xkb_symbols   { include "pc+us" };
+};

test/filecomp.c

@@ -48,6 +48,7 @@ main(void)
     assert(test_file(ctx, "keymaps/quartz.xkb"));
     assert(test_file(ctx, "keymaps/no-aliases.xkb"));
     assert(test_file(ctx, "keymaps/modmap-none.xkb"));
+    assert(test_file(ctx, "keymaps/invalid-escape-sequence.xkb"));
 
     assert(!test_file(ctx, "keymaps/divide-by-zero.xkb"));
     assert(!test_file(ctx, "keymaps/bad.xkb"));

tools/messages.c

@@ -48,6 +48,7 @@ static const struct xkb_message_entry xkb_messages[] = {
     {XKB_ERROR_WRONG_STATEMENT_TYPE, "Wrong statement type"},
     {XKB_WARNING_UNSUPPORTED_GEOMETRY_SECTION, "Unsupported geometry section"},
     {XKB_WARNING_CANNOT_INFER_KEY_TYPE, "Cannot infer key type"},
+    {XKB_WARNING_INVALID_ESCAPE_SEQUENCE, "Invalid escape sequence"},
     {XKB_WARNING_ILLEGAL_KEY_TYPE_PRESERVE_RESULT, "Illegal key type preserve result"},
     {XKB_ERROR_INVALID_INCLUDE_STATEMENT, "Invalid include statement"},
     {XKB_ERROR_INVALID_MODMAP_ENTRY, "Invalid modmap entry"},