nightt5879 opened a new pull request, #3505:
URL: https://github.com/apache/nuttx-apps/pull/3505
## Summary
Fixes #3109.
This PR bounds string handling in `system/settings` where the maximum field
sizes are already defined by Kconfig.
Commit structure:
- Commit 1 (`system/settings: Bound public string handling`) updates the
public settings paths to validate key/value/file strings with `strnlen()`
before scanning, compare fixed-size keys with `strncmp()`, and copy into
fixed-size fields with `strlcpy()`.
- Commit 2 (`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 and `snprintf()`, 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:
- Uses existing `CONFIG_SYSTEM_SETTINGS_KEY_SIZE`,
`CONFIG_SYSTEM_SETTINGS_VALUE_SIZE`, and `CONFIG_SYSTEM_SETTINGS_MAX_FILENAME`
limits.
- Does not change the settings API.
- Does not change the text or binary storage formats.
- Does not mechanically replace every string helper; changes are limited to
settings fields with known configured bounds.
## Impact
Settings input and storage-loaded string fields are no longer scanned or
compared beyond their configured maximum sizes.
- New feature: NO
- User adaptation required: NO intended user adaptation
- Build process change: NO
- Hardware/architecture/board change: NO
- Documentation update required: NO
- Security impact: YES, hardens bounded string handling in `system/settings`
- Compatibility impact: NO intended compatibility impact
## Testing
Host:
- Windows with WSL Ubuntu 24.04
- CPU: x86_64
- Compiler: GCC 13.3.0
Checks:
- `git diff --check upstream/master..HEAD`: pass
- `checkpatch.sh -c -u -m -g HEAD~2..HEAD`: pass
- `rg -n "strlen\(|strcmp\(|strncpy\(|strcpy\(|strcat\(" system/settings`:
no matches
- `sim:nsh` build with `CONFIG_SYSTEM_SETTINGS=y` and
`CONFIG_SYSTEM_SETTINGS_CACHED_SAVES` disabled: pass
- confirmed `CC: settings.c`
- confirmed `CC: storage_bin.c`
- confirmed `CC: storage_text.c`
- result: `SIM elf with dynamic libs archive in nuttx.tgz`
Note: the temporary test build disables
`CONFIG_SYSTEM_SETTINGS_CACHED_SAVES` because the base `sim:nsh` settings
configuration otherwise needs `SIGEV_THREAD` support, which is outside the
scope of this string-handling PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
