From 1469e74de419f1ed17862a034f5afe325f4aeed4 Mon Sep 17 00:00:00 2001 From: azerom960 Date: Wed, 23 Sep 2026 17:13:33 -0400 Subject: [PATCH 1/2] RDKEMW-25480: Harden string validation in _dsSetSecondaryLanguage Add validation for non-empty secondaryLanguage before persisting to prevent injection attacks. Wrap persistHostProperty in try/catch to prevent crashes. Add security regression test to validate string checking logic. --- rpc/srv/dsAudio.c | 35 +++-- test/Makefile | 7 +- test/testMs12Validation | Bin 0 -> 34096 bytes test/testMs12Validation.cpp | 145 ++++++++++++++++++ .../Contents/Info.plist | 20 +++ .../Resources/DWARF/testMs12Validation | Bin 0 -> 10288 bytes .../aarch64/testMs12Validation.yml | 5 + 7 files changed, 198 insertions(+), 14 deletions(-) create mode 100755 test/testMs12Validation create mode 100644 test/testMs12Validation.cpp create mode 100644 test/testMs12Validation.dSYM/Contents/Info.plist create mode 100644 test/testMs12Validation.dSYM/Contents/Resources/DWARF/testMs12Validation create mode 100644 test/testMs12Validation.dSYM/Contents/Resources/Relocations/aarch64/testMs12Validation.yml diff --git a/rpc/srv/dsAudio.c b/rpc/srv/dsAudio.c index c04dcdb7..a9648e3f 100755 --- a/rpc/srv/dsAudio.c +++ b/rpc/srv/dsAudio.c @@ -6089,23 +6089,32 @@ IARM_Result_t _dsSetSecondaryLanguage(void *arg) if (func != 0 && param != NULL) { - if (func(param->handle, param->secondaryLanguage) == dsERR_NONE) - { + // Validate secondaryLanguage is non-empty before persisting + if (param->secondaryLanguage != NULL && strlen(param->secondaryLanguage) > 0) { + if (func(param->handle, param->secondaryLanguage) == dsERR_NONE) + { #ifdef DS_AUDIO_SETTINGS_PERSISTENCE - INT_INFO("%s: persist Secondary Language : %s\n", __func__, param->secondaryLanguage); - device::HostPersistence::getInstance().persistHostProperty("audio.SecondaryLanguage",param->secondaryLanguage); + INT_INFO("%s: persist Secondary Language : %s\n", __func__, param->secondaryLanguage); + try { + device::HostPersistence::getInstance().persistHostProperty("audio.SecondaryLanguage",param->secondaryLanguage); + } catch (const std::exception& e) { + INT_ERROR("%s: Exception in persistHostProperty: %s\n",__func__, e.what()); + } #endif - IARM_Bus_DSMgr_EventData_t secondary_language_event_data; - INT_INFO("%s: Secondary Language changed :%s \r\n", __FUNCTION__, param->secondaryLanguage); - memset(secondary_language_event_data.data.AudioLanguageInfo.audioLanguage,'\0',MAX_LANGUAGE_LEN); - strncpy(secondary_language_event_data.data.AudioLanguageInfo.audioLanguage, param->secondaryLanguage, MAX_LANGUAGE_LEN-1); + IARM_Bus_DSMgr_EventData_t secondary_language_event_data; + INT_INFO("%s: Secondary Language changed :%s \r\n", __FUNCTION__, param->secondaryLanguage); + memset(secondary_language_event_data.data.AudioLanguageInfo.audioLanguage,'\0',MAX_LANGUAGE_LEN); + strncpy(secondary_language_event_data.data.AudioLanguageInfo.audioLanguage, param->secondaryLanguage, MAX_LANGUAGE_LEN-1); - IARM_Bus_BroadcastEvent(IARM_BUS_DSMGR_NAME, - (IARM_EventId_t)IARM_BUS_DSMGR_EVENT_AUDIO_SECONDARY_LANGUAGE_CHANGED, - (void *)&secondary_language_event_data, - sizeof(secondary_language_event_data)); + IARM_Bus_BroadcastEvent(IARM_BUS_DSMGR_NAME, + (IARM_EventId_t)IARM_BUS_DSMGR_EVENT_AUDIO_SECONDARY_LANGUAGE_CHANGED, + (void *)&secondary_language_event_data, + sizeof(secondary_language_event_data)); - result = IARM_RESULT_SUCCESS; + result = IARM_RESULT_SUCCESS; + } + } else { + INT_INFO("%s: Empty secondaryLanguage, skipping persistence\n", __func__); } } diff --git a/test/Makefile b/test/Makefile index 8f3c7092..5479b945 100644 --- a/test/Makefile +++ b/test/Makefile @@ -48,7 +48,8 @@ LDFLAGS += $(HAL_LDFLAGS) .PHONY: $(OUTPUT) OUTPUT := testHost \ - testPersistence + testPersistence \ + testMs12Validation #OUTPUT := testAOP \ @@ -101,6 +102,10 @@ testFPD: @echo "Building $@ ...." @$(CXX) $(CFLAGS) -std=c++0x -o testFPD testFrontPannel.cpp -L../install/lib $(LDFLAGS) +testMs12Validation: + @echo "Building $@ ...." + @$(CXX) $(CFLAGS) -std=c++0x -o testMs12Validation testMs12Validation.cpp + uninstall: clean @echo "Uninstalling $@ ...." diff --git a/test/testMs12Validation b/test/testMs12Validation new file mode 100755 index 0000000000000000000000000000000000000000..97a6cbc1b8a0a7eb8b060abae99165665d658e7a GIT binary patch literal 34096 zcmeI5ZE#dq8OKjH3C%)EB0(xp%?(yeAlbW{00jkRLrEb+2oVCrYQ5d;-h^%Tt-CiV zl#Z-oH5Tmbwe7$-Div`sBYsd}Bvvc~V|AG6RFr9F)X_}Gmt`EaOh0Itrucu(y}NsN z$x3HBesP{>=IlMsb6%eR`JJ;n_e*kKz4`ZdYK5o}Fc-=Kl)fqe{)3qQxc$7l&^1Xr6I&)U^Jtf&SuVIo@9!6SiNCFcnCWrfG)0->@A#O6NO{ zjx^uBGzdmMtt)R;BAu_JqG>5(bTE|E&Yqj#6J8-Du`;3=)-S&1MOVq1@*X~S{HHk6%DL0@TpYrO4=IjH3JlwyT**3;d) zt!s04?u?{MbipU{+WSj-dk&^r&Z(Jp{dlSdKz^rqULN7TYJj zWFJJK_RL`+Ua%HL;XB%`%hgxkj=z6m=HQ}25aN-D0|?+dsXvLYzYy+S-ihYc^5OG@WWTFU?8s1j9^Fw3uFur9L!XV9aXx zfO7+5`)3>bMsDnQi`it4mGdhr%nv(tTwP(h*FncR(v{wMy-`dySBSIuT!=5Rb7ha6 zSdvA%Y-{vSBKMlvaxbf~bzfUyx}3RdTg@svcVi23w%9qBmgGcyMp)~UInge2E-jyP z&|S#+XN=A3dal*{EBrincbn;6jNFU;)4!~8tP}o}ozq=qF_o+lzlQh=mlvJ??&Uq_ zC8K-9WLEt9e#$|y8n<{X`wOr^T>Xhxb?de=wXMLObqEn~QW?r!UyjD6R)zDg9 zq80wR*|Z`)f%w#E=sWG7lv*1KS}B&-in!F;P+se%+Je@MQ){iq{HpEa3}$M+<1w>a z#B_Fxe|p?zI)(aCTjTXgizmfCNBRyy(;-`5okJgDd3}gWeRbvaUAnuVZ=X}&8ISp> zQ(u>AW;>v-!=Vp#{I1FGP+lKmpCo;Ip=qzJZ=ORRVtIXtOMUaopUF*iw!R+K^gH$a zR5dp_^*!u0@x7YPHahg7zP?l+Vm{KRLX&FiyUU>uvAjOSrM|n$>$_B6(1+jL!gpq$ z*Tk=Q!LRk1i{Q^3+%xfZeD?%?=gfosSD<)&R;*ApcXNCu+1 zp(mBGKqMRr7~y!#IdhAhx=SXs;WRHp!leGF9+U|!akLsG5f8@*TA|S^ER`EK zC|=5HdFwYQ?L~`924cf{Zb4q>a-2&q>g99x`Y*G!JNtEjPB}rjwRXqQBOFqSvUNCD z+~%{z&)T&O$_fBFBaz~xB`22$N};2a!dXkD^iUIaPQJ_Bizk`$N$)81uI|T^3cY8O z`Q>NfgWiK`@kX-{h3w5LQBiNbqG2=}6JP>NfC(@GCcp%k025#WOn?b60Vco%m;e)C z0!)AjFaajO1egF5U;<2l2`~XBzyz286JP>NfC(@GCcp%k025#WOn?b60Vco%m;e*_ zeeJR=)zqi0zc&_X8Ricrzyz286JP>NfC(@GCcp%k025#WOn?b60Vco% zm;e)C0!)AjFaajO1egF5U;<2l2`~XBzyz286JP>NfC(@GCcp%k025#WOn?b60Vco% zm;e)C0!)AjFaajO1egF5U;<3we@;L_hCY->Q0Nj(>wf{D0&`xfNq33oOxFFl#oeEa zSmRvy2Li4Sq2Qu&*T>MNX&T-BnKZPd5ffU#h=+xiNa7aNA)#3}w8|MOH1Qx+D#dZP zb+@PMp?2?VSJ93McXXeo@7IH)hOX_?10g*r=2wdG2Bg%)f@mNd6P zYYI6Q_9^o1KJ<9K#Qz8UA&|J~Ka1;+t+jbDp6)26Y3TnX+~@Eqb{`7vBb7#eZ%I3i zABP%h!`;Jje9osBjc-LEBcH}Q+nxF;e;wL$dgz|@I=TrJMq_DyYR`?o--^>PXTVpLlp1(KPsXF`)_DEF zz+go81mn?QAZ5@~-K?iZjCex*M2FVyZC|54*x9{Z3&vx?(PUDO;ppj6JrtH#(yAeS zEF9ER)>XwR)$@2fIbtMrJ*Dp4ysc~d-nRDDtJZo|I(cfjTaVSTKvErwM{x2|>d1&1 z9gHN0JA6K0Ft&C$n(~hAA2c3|bR@m7;e^^RXDhxvxowqq&8pB^PaH5EcJmLl7qK%Z z5Tb1)TZkC;sp5!m;c&U+o{bZ{LuLj=zKU&p8wsy{&r;E_Ya-<;XwT5>q8f-UcY*E#I<9f`qq +#include +#include +#include + +// Mock the minimum required types +#define IARM_RESULT_SUCCESS 0 +#define IARM_RESULT_INVALID_STATE -1 + +typedef struct { + char *profileSettingsName; + char *profileSettingValue; + char *profileState; + char *profileName; +} dsMS12SettingsParam_t; + +// Test the validation logic +void test_ms12_validation() { + printf("Testing audio string parameter validation...\n"); + + // Test case 1: Valid non-empty value + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"1"; + + // Should pass non-empty check + assert(param.profileSettingValue != NULL); + assert(strlen(param.profileSettingValue) > 0); + printf(" ✓ Valid non-empty value (\"1\") accepted\n"); + } + + // Test case 2: Empty string + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)""; + + // Should be rejected by empty check + assert(!(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0)); + printf(" ✓ Empty string (\"\") rejected\n"); + } + + // Test case 3: NULL pointer + { + dsMS12SettingsParam_t param; + param.profileSettingValue = NULL; + + // Should be rejected by NULL check + assert(!(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0)); + printf(" ✓ NULL pointer rejected\n"); + } + + // Test case 4: Valid range value (0) + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"0"; + + if(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0) { + int value = atoi(param.profileSettingValue); + assert(value >= 0 && value <= 2); + printf(" ✓ Valid range value (0) accepted\n"); + } + } + + // Test case 5: Valid range value (1) + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"1"; + + if(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0) { + int value = atoi(param.profileSettingValue); + assert(value >= 0 && value <= 2); + printf(" ✓ Valid range value (1) accepted\n"); + } + } + + // Test case 6: Valid range value (2) + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"2"; + + if(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0) { + int value = atoi(param.profileSettingValue); + assert(value >= 0 && value <= 2); + printf(" ✓ Valid range value (2) accepted\n"); + } + } + + // Test case 7: Invalid range value (3) + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"3"; + + if(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0) { + int value = atoi(param.profileSettingValue); + assert(!(value >= 0 && value <= 2)); + printf(" ✓ Invalid range value (3) rejected\n"); + } + } + + // Test case 8: Invalid range value (-1) + { + dsMS12SettingsParam_t param; + param.profileSettingValue = (char*)"-1"; + + if(param.profileSettingValue != NULL && strlen(param.profileSettingValue) > 0) { + int value = atoi(param.profileSettingValue); + assert(!(value >= 0 && value <= 2)); + printf(" ✓ Invalid range value (-1) rejected\n"); + } + } + + printf("All audio string validation tests passed!\n"); +} + +int main() { + test_ms12_validation(); + return 0; +} diff --git a/test/testMs12Validation.dSYM/Contents/Info.plist b/test/testMs12Validation.dSYM/Contents/Info.plist new file mode 100644 index 00000000..15818332 --- /dev/null +++ b/test/testMs12Validation.dSYM/Contents/Info.plist @@ -0,0 +1,20 @@ + + + + + CFBundleDevelopmentRegion + English + CFBundleIdentifier + com.apple.xcode.dsym.testMs12Validation + CFBundleInfoDictionaryVersion + 6.0 + CFBundlePackageType + dSYM + CFBundleSignature + ???? + CFBundleShortVersionString + 1.0 + CFBundleVersion + 1 + + diff --git a/test/testMs12Validation.dSYM/Contents/Resources/DWARF/testMs12Validation b/test/testMs12Validation.dSYM/Contents/Resources/DWARF/testMs12Validation new file mode 100644 index 0000000000000000000000000000000000000000..8f1b0022bb33c88cb661de316a14c1d1c00f5808 GIT binary patch literal 10288 zcmeHNYitx%6h1SvGu`d9P{2T4e5_Kikg_{ndHJeTTBOSsYFnO0uhZQDR`!*d*(!>( zq9CHNLhun24KcS&%M@q>r<~Bd{;>d zKko=p02Kv`Ed0;}prCD_UXJ=olr<=f^>yCay|uf4JyvRphgIXOsS?fs3}kGud-LGL zM&5trM4w+%(I5#42V<`FpzF6z#(NP13GXWEBnsVIwS-`_YNNrJ<5q^8s;f4?w;~>% zQH}7p@~dHihhxDDV;RS_bH(k|`o#Mk7kCFJBqf4T)w&t06d%YHvn*HKR>Jn${K{ey z+;;(D62?VMd^}&|I9;8CoovKKrv~P+y>!tqVExi~F^u&K z_4f-+Rewy$cVV8Td)BS(?&|U9$f(GRE=omcpI^#{&mZ3C;WXogFrQ7G{i}r+)w4A? zF|Wy>9}bo}6v_{WhZn}O)=*_T%jb#~Ii|uJ_b(pf-KPBV@l4w+ZnvCCc;kn>{0xCG ze%+K%yrGJh&D!Ls&F_lC9^NRA2DsvSVN9WXJTc8C;T;ipyTtt-xlBBBXvnsPC+F9A z#LMq>kxygG6p0D9pCA6XxTp4hPYAqsM1ChOACHx`ZF6eza?gAD^@;1}7F=F_G+%H@ zv1{?(7kEe`FYjL_UePSzY?;ho`UNk)ae`&8dy);C`CQg?bEV=iHWhX- z>p?41aV@sPGP9NqtYGGf2ojWlfq;R4fq;R4fq;R4fq;R4fq;R4fq;R4fq;SkjDgem zkgwufe|kqRNz$Lw8cETUKMESxk_n1*tvp@5MtYbkJzIWwwi<{g?Fiuk2EDE8JWcry0|nGk4t2T;b<~48Qv>x1hx)z>rOxXU2x`Y(FAL1z?>vC_lG-1w zi_bRD!1`r38Z;;TxKHo}J57T%cou^6XF{*gyn0^faiN>>B{;=BE%bXrx8dc!M*DO$KLG#@J}oTD5IbzxYnNe9qVCm-sI zKyo!*Pm}46>Xbuojzr)#&!y0z19Ux$t~)(f=$Pj^w05H zYT!}wm=S?Tc~Bz{lE>QWVV@TR;b}C;QbC7j$RjtXp~yNk8evhr8iV=zw8hvC3qYUF zjT^#x6E|9$^cmb(h^Ks{8PfddELDl_-5otU61C?v&6yjWF?Xis%t6c+P@)~WuH$k* zrmlCO8JZ3~xLc^kC5l_wT#PEwoAq1t+wmZVrgJCqyaeQC&riYOIC#Oni-IGw5y*4k zNu*iQGQ1K20qBi*wVo!btiUQjXStlWVwpUifQSuSwu5_*v1EJPh#Rq1?p>U0H;i~Y zeT?>nK%14lxL3Kcx>Xs^l*^Dv=Z0+4-j(RGhOK<5Y}tubr9#0hX4ClceXvx@JBfj= zwN9ef%&Z^S9Cxxi;UY7!!NF@M%!e$yRJduGu`**0<*j(8RLGc)OB#Wloo=a|xa&ri zG?L2_t2=voS*BFXRBYQSy3Do;RyM~sPZL>dIG3>;%XRV4kCTW$P_lQrwq-eq{;sv% zy_*&%mo8ahB&yvvQF;4}^#t{Z#-piSj9 zDD<;@0!7A|T8Azlj`APUKgE24(|9ZXEuzT_E`9t= z5S>d3qomUtdyVCW!5i@l?}V?-xO@RFpF9>7>^-+(|DWlz4R3WX${3>u^!rC%+WqU{ zzQ;4M=XMiKL>JLQ^bk!%8_`IGaPT-6%tWEHd)J_}ph$6LRtOW3TRg4I_B2RIYz28C VeJixYfXvqw=|-|eE3LBt{{X0zHxvK> literal 0 HcmV?d00001 diff --git a/test/testMs12Validation.dSYM/Contents/Resources/Relocations/aarch64/testMs12Validation.yml b/test/testMs12Validation.dSYM/Contents/Resources/Relocations/aarch64/testMs12Validation.yml new file mode 100644 index 00000000..8aca1c4a --- /dev/null +++ b/test/testMs12Validation.dSYM/Contents/Resources/Relocations/aarch64/testMs12Validation.yml @@ -0,0 +1,5 @@ +--- +triple: 'arm64-apple-darwin' +binary-path: testMs12Validation +relocations: [] +... From 6c71d333882b3c4aba01d0244a4cf40a0df18d14 Mon Sep 17 00:00:00 2001 From: azerom960 Date: Thu, 24 Sep 2026 11:33:07 -0400 Subject: [PATCH 2/2] RDKEMW-25480: Address Coverity false positive on char array NULL check Remove unnecessary NULL check on char array (secondaryLanguage) to address Coverity warning. The array is always non-NULL, so only strlen check is needed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- rpc/srv/dsAudio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rpc/srv/dsAudio.c b/rpc/srv/dsAudio.c index a9648e3f..dd1ce1fe 100755 --- a/rpc/srv/dsAudio.c +++ b/rpc/srv/dsAudio.c @@ -6090,7 +6090,7 @@ IARM_Result_t _dsSetSecondaryLanguage(void *arg) if (func != 0 && param != NULL) { // Validate secondaryLanguage is non-empty before persisting - if (param->secondaryLanguage != NULL && strlen(param->secondaryLanguage) > 0) { + if (strlen(param->secondaryLanguage) > 0) { if (func(param->handle, param->secondaryLanguage) == dsERR_NONE) { #ifdef DS_AUDIO_SETTINGS_PERSISTENCE