config: bound conf_parse_name() loop to CONF_MAX_DIR_DEPTH

[94xhn]

Problem

conf_parse_name() in sys/config/src/config.c tokenizes a config
name string on / and writes each token pointer into the caller's
name_argv[] array with no bound check:

int
conf_parse_name(char *name, int *name_argc, char *name_argv[])
{
    char *tok;
    char *tok_ptr;
    char *sep = CONF_NAME_SEPARATOR;
    int i;

    tok = strtok_r(name, sep, &tok_ptr);

    i = 0;
    while (tok) {
        name_argv[i++] = tok;
        tok = strtok_r(NULL, sep, &tok_ptr);
    }
    *name_argc = i;

    return 0;
}

Every real caller declares a fixed 8-entry array
(CONF_MAX_DIR_DEPTH, sys/config/include/config/config.h:39) on the
stack and passes it straight in:

• conf_set_value() — sys/config/src/config.c:447
• conf_get_value() — sys/config/src/config.c:475
• conf_commit() — sys/config/src/config.c:497
• conf_save_tree() — sys/config/src/config_store.c:264

A name with more than 8 /-separated segments overflows name_argv[]
and writes past the end of the array on the stack, corrupting whatever
local variables/saved registers follow it.

This is reachable remotely, not just from local misuse: mgmt/config
(sys/config/src/config_mgmt.c, mcumgr "config read"/"config write")
copies an attacker-supplied CBOR "name" string of up to
CONF_MAX_NAME_LEN (64) bytes into a local buffer and passes it
unvalidated (for segment count) into conf_get_value() /
conf_set_value(). A name such as 16 single-character segments
("a/a/a/a/a/a/a/a/a/a/a/a/a/a/a/a", 31 bytes — well under the 64-byte
limit) is enough to write 8 pointers past the end of the 8-slot array.
The same unbounded loop is also reachable locally via the config shell
command, since the defect is in the shared conf_parse_name() /
conf_parse_and_lookup() core, not in config_mgmt-specific code.

Fix

Bound the tokenizer loop and return an error (-1) once
CONF_MAX_DIR_DEPTH segments have been collected, instead of writing
past the array:

    i = 0;
    while (tok) {
        if (i >= CONF_MAX_DIR_DEPTH) {
            return -1;
        }
        name_argv[i++] = tok;
        tok = strtok_r(NULL, sep, &tok_ptr);
    }
    *name_argc = i;

No other code needs to change: conf_parse_and_lookup()
(sys/config/src/config.c:151-160) already does
if (rc) { return NULL; }, and every one of the four callers already
treats a NULL lookup result the same way it treats "handler not
found" — returning OS_INVALID_PARM (or, for conf_commit(NULL),
simply skipping that entry). So an over-depth name is now rejected the
same way an unregistered/garbled name already was, with no behavior
change for any name that has ≤ 8 segments (including the exact
boundary case of 8 segments, which still parses correctly).

Verification

I do not have the full Mynewt build toolchain set up to build a target
image, so I verified the change with a standalone C harness (gcc 8.1.0,
-std=c99 -Wall -Wextra -O0, zero warnings) built from the function
body extracted verbatim (via awk) from the actual patched
sys/config/src/config.c, driven through a struct reproducing the real
stack layout (name_argc + name_argv[CONF_MAX_DIR_DEPTH] + an
adjacent canary standing in for the real neighboring locals):

CONF_MAX_DIR_DEPTH = 8

[normal 3-segment name       ] rc=0  argc=3   canary_intact=yes
[exactly 8 segments          ] rc=0  argc=8   canary_intact=yes
[9 segments (1 over)         ] rc=-1 argc=-1  canary_intact=yes
[16-segment attacker name    ] rc=-1 argc=-1  canary_intact=yes
[functional parse sys/a/b/c  ] rc=0 argc=4 -> CORRECT

=== RESULT: ALL CHECKS PASSED against the ACTUAL patched config.c function ===

I additionally ran the identical harness against an unpatched copy of
conf_parse_name() beforehand and confirmed the 16-segment case
corrupts the canary (canary_intact=NO (BUG!)) while the 3-segment
normal case does not — matching the described failure mode.

Disclosure

Generative AI (Claude, Anthropic) was used to help investigate this
issue and implement/verify the fix. All findings and the change itself
were reviewed by me before submission.