From mboxrd@z Thu Jan 1 00:00:00 1970 Subject: MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c does not initialize AccessResults in HiiConfigRoutingExportConfig() To: devel@edk2.groups.io From: "Charles Hyde" X-Originating-Location: Melbourne, Florida, US (184.90.249.149) X-Originating-Platform: Windows Firefox 114 User-Agent: GROUPS.IO Web Poster MIME-Version: 1.0 Date: Sat, 01 Jul 2023 15:38:22 -0700 Message-ID: <1o5K.1688251102466542614.Xexl@groups.io> X-Groupsio-MsgNum: 106615 Content-Type: multipart/mixed; boundary="Kx73jLx3c7vYgtd4Plkq" --Kx73jLx3c7vYgtd4Plkq Content-Type: multipart/alternative; boundary="JigzZvzA6iY5RqteXVKX" --JigzZvzA6iY5RqteXVKX Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable In the function HiiConfigRoutingExportConfig() in MdeModulePkg/Universal/Hi= iDatabaseDxe/ConfigRouting.c, the string pointer AccessResults is not initi= alized like the other pointers are. This variable should be set to NULL nea= r line 5295, along with the other pointers. In testing my company=E2=80=99s BIOS with uefi-sct, I found strange behavio= r that I traced to this variable not being initialized before calling Confi= gAccess->ExtractConfig() at line 5322. If you check lines 4875 through 4888= , you will find this other function does initialize its pointer variables. Also, AccessResults is not checked for NULL before it is used at line 5357.= This could result in StrStr() and GetElementsFromRequest() both being pass= ed a NULL pointer and this section of code would be invalid. This attached patch solves both problems, it initializes the pointer to NUL= L, and it checks for NULL after calling ConfigAccess->ExtractConfig(). As a= result of implementing my patch, my company's BIOS passed the uefi-sct wit= hout issue. I also checked a much older BIOS that we have, and the problem = exists there as well. It appears this uninitialized pointer has been around= for no less than 10 years. --JigzZvzA6iY5RqteXVKX Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: quoted-printable In the function HiiConfigRoutingExportConfig() in MdeModulePkg/Universal/Hi= iDatabaseDxe/ConfigRouting.c, the string pointer AccessResults is not initi= alized like the other pointers are. This variable should be set to NULL nea= r line 5295, along with the other pointers.

In testing my compan= y’s BIOS with uefi-sct, I found strange behavior that I traced to thi= s variable not being initialized before calling ConfigAccess->ExtractCon= fig() at line 5322. If you check lines 4875 through 4888, you will find thi= s other function does initialize its pointer variables.

Also, Ac= cessResults is not checked for NULL before it is used at line 5357. This co= uld result in StrStr() and GetElementsFromRequest() both being passed a NUL= L pointer and this section of code would be invalid.

This attach= ed patch solves both problems, it initializes the pointer to NULL, and it c= hecks for NULL after calling ConfigAccess->ExtractConfig(). As a result = of implementing my patch, my company's BIOS passed the uefi-sct without iss= ue. I also checked a much older BIOS that we have, and the problem exists t= here as well. It appears this uninitialized pointer has been around for no = less than 10 years. --JigzZvzA6iY5RqteXVKX-- --Kx73jLx3c7vYgtd4Plkq Content-Type: application/octet-stream; name="patch.diff" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patch.diff" ZGlmZiAtLWdpdCBhL01kZU1vZHVsZVBrZy9Vbml2ZXJzYWwvSGlpRGF0YWJhc2VEeGUvQ29uZmln Um91dGluZy5jIGIvTWRlTW9kdWxlUGtnL1VuaXZlcnNhbC9IaWlEYXRhYmFzZUR4ZS9Db25maWdS b3V0aW5nLmMKaW5kZXggNWFlNjE4OWEyOC4uNzAwZTgwNjE5YiAxMDA2NDQKLS0tIGEvTWRlTW9k dWxlUGtnL1VuaXZlcnNhbC9IaWlEYXRhYmFzZUR4ZS9Db25maWdSb3V0aW5nLmMKKysrIGIvTWRl TW9kdWxlUGtnL1VuaXZlcnNhbC9IaWlEYXRhYmFzZUR4ZS9Db25maWdSb3V0aW5nLmMKQEAgLTQy MCwxNCArNDIwLDE5IEBAIEFwcGVuZFRvTXVsdGlTdHJpbmcgKAogICB9DQogDQogICBBcHBlbmRT dHJpbmdTaXplID0gU3RyU2l6ZSAoQXBwZW5kU3RyaW5nKTsNCisgIGlmIChBcHBlbmRTdHJpbmdT aXplIDw9IHNpemVvZihDSEFSMTYpKQ0KKyAgICByZXR1cm4gRUZJX1NVQ0NFU1M7DQorDQogICBN dWx0aVN0cmluZ1NpemUgID0gU3RyU2l6ZSAoKk11bHRpU3RyaW5nKTsNCiAgIE1heExlbiAgICAg ICAgICAgPSBNQVhfU1RSSU5HX0xFTkdUSCAvIHNpemVvZiAoQ0hBUjE2KTsNCiANCiAgIC8vDQog ICAvLyBFbmxhcmdlIHRoZSBidWZmZXIgZWFjaCB0aW1lIHdoZW4gbGVuZ3RoIGV4Y2VlZHMgTUFY X1NUUklOR19MRU5HVEguDQogICAvLw0KLSAgaWYgKChNdWx0aVN0cmluZ1NpemUgKyBBcHBlbmRT dHJpbmdTaXplID4gTUFYX1NUUklOR19MRU5HVEgpIHx8DQotICAgICAgKE11bHRpU3RyaW5nU2l6 ZSA+IE1BWF9TVFJJTkdfTEVOR1RIKSkNCisgIGlmICgoTXVsdGlTdHJpbmdTaXplICsgQXBwZW5k U3RyaW5nU2l6ZSA+IE1BWF9TVFJJTkdfTEVOR1RIKSAvKnx8DQorICAgICAgKE11bHRpU3RyaW5n U2l6ZSA+IE1BWF9TVFJJTkdfTEVOR1RIKSovKSAgLy8gVGhlcmUgaXMgbm8gbmVlZCB0byBjaGVj ayB0aGUgc2Vjb25kIHBhcnQuDQorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgLy8gSWYgdGhlIGZpcnN0IHBhcnQgaXMgZmFsc2UsIHRoZSBzZWNvbmQgcGFy dCB3aWxsIGFsd2F5cyBiZSBmYWxzZS4NCisgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAvLyBJZiB0aGUgc2Vjb25kIHBhcnQgaXMgdHJ1ZSwgdGhlIGZpcnN0 IHBhcnQgbXVzdCBhbHNvIGJlIHRydWUuDQogICB7DQogICAgICpNdWx0aVN0cmluZyA9IChFRklf U1RSSU5HKVJlYWxsb2NhdGVQb29sICgNCiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICBNdWx0aVN0cmluZ1NpemUsDQpAQCAtNTI5Miw2ICs1Mjk3LDcgQEAgSGlpQ29uZmlnUm91dGlu Z0V4cG9ydENvbmZpZyAoCiAgICAgLy8NCiAgICAgSWZyRGF0YVBhcnNlZEZsYWcgPSBGQUxTRTsN CiAgICAgUHJvZ3Jlc3MgICAgICAgICAgPSBOVUxMOw0KKyAgICBBY2Nlc3NSZXN1bHRzICAgICA9 IE5VTEw7DQogICAgIEhpaUhhbmRsZSAgICAgICAgID0gTlVMTDsNCiAgICAgRGVmYXVsdFJlc3Vs dHMgICAgPSBOVUxMOw0KICAgICBEYXRhYmFzZSAgICAgICAgICA9IE5VTEw7DQpAQCAtNTMyNiw2 ICs1MzMyLDE3IEBAIEhpaUNvbmZpZ1JvdXRpbmdFeHBvcnRDb25maWcgKAogICAgICAgICAgICAg ICAgICAgICAgICAgICAgICAmQWNjZXNzUmVzdWx0cw0KICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgKTsNCiAgICAgaWYgKEVGSV9FUlJPUiAoU3RhdHVzKSkgew0KKw0KKyAgICAgIC8vIElm IGFuIGVycm9yIHdhcyByZXR1cm5lZCwgdGhlbiBkbyBub3QgYmVsaWV2ZSBhbnkgcmVzdWx0cyBp biB0aGVzZSB0d28gcG9pbnRlcnMuDQorICAgICAgaWYgKFByb2dyZXNzKSB7DQorICAgICAgICBG cmVlUG9vbCAoUHJvZ3Jlc3MpOw0KKyAgICAgICAgUHJvZ3Jlc3MgPSBOVUxMOw0KKyAgICAgIH0N CisgICAgICBpZiAoQWNjZXNzUmVzdWx0cykgew0KKyAgICAgICAgRnJlZVBvb2wgKEFjY2Vzc1Jl c3VsdHMpOw0KKyAgICAgICAgQWNjZXNzUmVzdWx0cyA9IE5VTEw7DQorICAgICAgfQ0KKw0KICAg ICAgIC8vDQogICAgICAgLy8gVXBkYXRlIEFjY2Vzc1Jlc3VsdHMgYnkgZ2V0dGluZyBkZWZhdWx0 IHNldHRpbmcgZnJvbSBJRlIgd2hlbiBIaWlQYWNrYWdlIGlzIHJlZ2lzdGVyZWQgdG8gSGlpSGFu ZGxlDQogICAgICAgLy8NCkBAIC01MzUwLDYgKzUzNjcsMTggQEAgSGlpQ29uZmlnUm91dGluZ0V4 cG9ydENvbmZpZyAoCiAgICAgfQ0KIA0KICAgICBpZiAoIUVGSV9FUlJPUiAoU3RhdHVzKSkgew0K Kw0KKyAgICAgIC8vIElmIEFjY2Vzc1Jlc3VsdHMgPT0gTlVMTCwgdGhlcmUgaXMgbm90aGluZyB0 byBiZSBkb25lLg0KKyAgICAgIGlmIChBY2Nlc3NSZXN1bHRzID09IE5VTEwpIHsNCisgICAgICAg IGlmIChQcm9ncmVzcykNCisgICAgICAgICAgRnJlZVBvb2wgKFByb2dyZXNzKTsNCisNCisgICAg ICAgIGlmIChDb25maWdSZXF1ZXN0ICE9IE5VTEwpDQorICAgICAgICAgIEZyZWVQb29sIChDb25m aWdSZXF1ZXN0KTsNCisNCisgICAgICAgIGNvbnRpbnVlOw0KKyAgICAgIH0NCisNCiAgICAgICAv Lw0KICAgICAgIC8vIFVwZGF0ZSBBY2Nlc3NSZXN1bHRzIGJ5IGdldHRpbmcgZGVmYXVsdCBzZXR0 aW5nIGZyb20gSUZSIHdoZW4gSGlpUGFja2FnZSBpcyByZWdpc3RlcmVkIHRvIEhpaUhhbmRsZQ0K ICAgICAgIC8vDQo= --Kx73jLx3c7vYgtd4Plkq--