fix(studio): Ensure null-termination of layer name read from settings #2478
Conversation
app/src/keymap.c
Outdated
| @@ -793,6 +793,8 @@ static int keymap_handle_set(const char *name, size_t len, settings_read_cb read | |||
| LOG_ERR("Failed to handle keymap layer name from settings (err %d)", err); | |||
| return err; | |||
| } | |||
|
|
|||
| zmk_keymap_layer_names[layer][len] = 0; | |||
There was a problem hiding this comment.
We need to be careful just in case someone has changed the CONFIG_ZMK_KEYMAP_LAYER_NAME_MAX_LEN value and lowered it since a setting was stored. This should probably be:
| zmk_keymap_layer_names[layer][len] = 0; | |
| zmk_keymap_layer_names[layer][MIN(len, CONFIG_ZMK_KEYMAP_LAYER_NAME_MAX_LEN - 1)] = 0; |
To be safe.
There was a problem hiding this comment.
Maybe using zmk_keymap_layer_names[layer][err] would be better? According to the comment in settings_read_cb, it represents the number of bytes read.
There was a problem hiding this comment.
The length of the stored string may be longer than the allocated memory into which we are loading it. So we need to be more conservative. See the previous comment.
There was a problem hiding this comment.
Yes, but theoretically, err shouldn’t be longer than the length passed to read_cb, which is already MIN(len, CONFIG_ZMK_KEYMAP_LAYER_NAME_MAX_LEN - 1).
There was a problem hiding this comment.
Oh indeed! That should be fine.
This fixes the string leak when a layer name is changed to a longer one, but is discarded and reverted to the original shorter one from ZMK Studio.
xingrz
force-pushed
the
pr/fix-layer-name-null-termi
branch
from
8aef1a1
to
7a697a5
Compare
This fixes the string leak when a layer name is changed to a longer one, but is discarded and reverted to the original shorter one from ZMK Studio.