RDKEMW-15199:[SECVULN] Command injection via /opt/.telemetry/dca_temp…#355
Conversation
…_file.conf Signed-off-by: PriyaDharshini_Kathiravan <priyakathiravan05@gmail.com>
There was a problem hiding this comment.
Pull request overview
Addresses a reported command-injection vector by adding validation/sanitization for XConf-provided top_log.txt marker search strings before accepting them into the profile.
Changes:
- Added
<ctype.h>dependency and introducedsanitize_string()helper. - Added validation check for
TopMarker(top_log.txt)ref/search string inaddParameter().
| if(sanitize_string(strdup(ref)) != 0) | ||
| { | ||
| T2Error("Parameter can't be added as invalid search string encountered\n"); | ||
| return T2ERROR_FAILURE; | ||
| } |
There was a problem hiding this comment.
sanitize_string(strdup(ref)) can crash on OOM because strdup(ref) may return NULL and sanitize_string() dereferences its argument. Also, since you only need to validate characters, avoid allocating/freeing altogether by validating ref directly (or fail early if ref is empty/invalid) before storing it in tMarker->searchString.
| if(sanitize_string(strdup(ref)) != 0) | ||
| { | ||
| T2Error("Parameter can't be added as invalid search string encountered\n"); | ||
| return T2ERROR_FAILURE; | ||
| } |
There was a problem hiding this comment.
New security behavior (rejecting invalid top_log.txt search strings) should be covered by unit tests to prevent regressions. There are existing gtests for XConf parsing (e.g., source/test/t2parser/t2parserxconfTest.cpp), but none assert that a profile with an invalid content/search string is rejected.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Bypassing L2 failures and merging as the test failures are not from these changes. Test failures appear to be due to test docker infrastructure changes from recent release - https://github.com/rdkcentral/docker-device-mgt-service-test/releases/tag/1.7.13 |
…_file.conf