system/settings: Bound settings string handling#3505
Open
nightt5879 wants to merge 2 commits into
Open
Conversation
Use strnlen() for public key, value, and storage path length checks so user-provided settings strings are validated against the configured maximum sizes before they are scanned. Use bounded key comparisons and strlcpy() for fixed-size settings fields. This addresses part of apache#3109 without changing the settings API or storage formats. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
Use configured key, value, and filename limits while loading and saving settings storage data. The text backend now builds backup filenames with a sized buffer and bounded formatting, and both text and binary loading reject keys or string values that are not terminated within their configured field sizes. This completes apache#3109 without changing the storage formats. Signed-off-by: Nightt <87569709+nightt5879@users.noreply.github.com>
xiaoxiang781216
approved these changes
May 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3109.
This PR bounds string handling in
system/settingswhere the maximum field sizes are already defined by Kconfig.Commit structure:
system/settings: Bound public string handling) updates the public settings paths to validate key/value/file strings withstrnlen()before scanning, compare fixed-size keys withstrncmp(), and copy into fixed-size fields withstrlcpy().system/settings: Bound storage string handling) applies the same bounds to text and binary storage loading/saving. Text storage backup filenames are built with a sized buffer andsnprintf(), and storage-loaded keys/string values are rejected if they are not terminated within their configured field sizes.The second commit is logically separable. I am happy to drop it if maintainers prefer to limit this PR to the public settings API paths only.
Scope:
CONFIG_SYSTEM_SETTINGS_KEY_SIZE,CONFIG_SYSTEM_SETTINGS_VALUE_SIZE, andCONFIG_SYSTEM_SETTINGS_MAX_FILENAMElimits.Impact
Settings input and storage-loaded string fields are no longer scanned or compared beyond their configured maximum sizes.
system/settingsTesting
Host:
Checks:
git diff --check upstream/master..HEAD: passcheckpatch.sh -c -u -m -g HEAD~2..HEAD: passrg -n "strlen\(|strcmp\(|strncpy\(|strcpy\(|strcat\(" system/settings: no matchessim:nshbuild withCONFIG_SYSTEM_SETTINGS=yandCONFIG_SYSTEM_SETTINGS_CACHED_SAVESdisabled: passCC: settings.cCC: storage_bin.cCC: storage_text.cSIM elf with dynamic libs archive in nuttx.tgzNote: the temporary test build disables
CONFIG_SYSTEM_SETTINGS_CACHED_SAVESbecause the basesim:nshsettings configuration otherwise needsSIGEV_THREADsupport, which is outside the scope of this string-handling PR.