public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* HiiValidateSettings issue with string item
@ 2017-08-24  6:59 Wim Vervoorn
  2017-08-24  8:35 ` Bi, Dandan
  0 siblings, 1 reply; 2+ messages in thread
From: Wim Vervoorn @ 2017-08-24  6:59 UTC (permalink / raw)
  To: edk2-devel@lists.01.org

Hello,

I am running into an issue with HiiValidateSettings (); when my VFR contains a string item that is filled with a string of maximum length. In this case the validation returns an error because it thinks the string is too long.

During the validation ValidateQuestionFromVfr (); checks if the stringlength is valid. The issue is that this uses the "maxsize" value * 2 from the VFR. It does this using the StrSize function which includes the trailing terminator. This is of course correct. The maxsize from the VFR indicates only the amount of characters excluding the terminator.

As a quickfix I changed the ValidateQuestionFromVfr () to take this into account but I am doubting if this is the correct solution. Can you shed some light here?

Below is the fragment where I see this issue:

            //
            // Get Offset/Width by Question header and OneOf Flags
            //
            Offset = IfrString->Question.VarStoreInfo.VarOffset;
            //
            // Check whether this question is in current block array.
            //
            if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
              //
              // This question is not in the current configuration string. Skip it.
              //
              break;
            }
            //
            // Check this var question is in the var storage
            //
            if ((Offset + Width) > VarStoreData.Size) {
              //
              // This question exceeds the var store size.
              //
                  return EFI_INVALID_PARAMETER;
            }

            //
            // Check current string length is less than maxsize
            //
            // Please note we subtract sizeof(CHAR16) here because the StrSize returns the length including the terminator
            // while we specify the length in characters in the VFR!
            //
ORG ->                 // if ( (StrSize ((CHAR16 *) (VarBuffer + Offset)) > Width) {
CHANGED->            if ( (StrSize ((CHAR16 *) (VarBuffer + Offset)) - sizeof(CHAR16)) > Width) {
                  return EFI_INVALID_PARAMETER;
            }
          }
          break;
Best Regards,
Wim Vervoorn

Eltan B.V.
Ambachtstraat 23
5481 SM Schijndel
The Netherlands

T : +31-(0)73-594 46 64
E : wvervoorn@eltan.com
W : http://www.eltan.com<http://www.eltan.com/>
"THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES."




^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: HiiValidateSettings issue with string item
  2017-08-24  6:59 HiiValidateSettings issue with string item Wim Vervoorn
@ 2017-08-24  8:35 ` Bi, Dandan
  0 siblings, 0 replies; 2+ messages in thread
From: Bi, Dandan @ 2017-08-24  8:35 UTC (permalink / raw)
  To: Wim Vervoorn, edk2-devel@lists.01.org

Hi Wim Vervoorn,

Thanks for reporting this issue. It's a bug of current code and your solution can fix it correctly. 
But when the storage is NameValueType, for string opcode, current code also calculate the string length incorrectly, the length should not include the trailing terminator and storage name. 
So could you help to file a bug on bugzilla and then we can fix these issues?
https://bugzilla.tianocore.org/

Thanks,
Dandan

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wim Vervoorn
Sent: Thursday, August 24, 2017 2:59 PM
To: edk2-devel@lists.01.org
Subject: [edk2] HiiValidateSettings issue with string item

Hello,

I am running into an issue with HiiValidateSettings (); when my VFR contains a string item that is filled with a string of maximum length. In this case the validation returns an error because it thinks the string is too long.

During the validation ValidateQuestionFromVfr (); checks if the stringlength is valid. The issue is that this uses the "maxsize" value * 2 from the VFR. It does this using the StrSize function which includes the trailing terminator. This is of course correct. The maxsize from the VFR indicates only the amount of characters excluding the terminator.

As a quickfix I changed the ValidateQuestionFromVfr () to take this into account but I am doubting if this is the correct solution. Can you shed some light here?

Below is the fragment where I see this issue:

            //
            // Get Offset/Width by Question header and OneOf Flags
            //
            Offset = IfrString->Question.VarStoreInfo.VarOffset;
            //
            // Check whether this question is in current block array.
            //
            if (!BlockArrayCheck (CurrentBlockArray, Offset, Width)) {
              //
              // This question is not in the current configuration string. Skip it.
              //
              break;
            }
            //
            // Check this var question is in the var storage
            //
            if ((Offset + Width) > VarStoreData.Size) {
              //
              // This question exceeds the var store size.
              //
                  return EFI_INVALID_PARAMETER;
            }

            //
            // Check current string length is less than maxsize
            //
            // Please note we subtract sizeof(CHAR16) here because the StrSize returns the length including the terminator
            // while we specify the length in characters in the VFR!
            //
ORG ->                 // if ( (StrSize ((CHAR16 *) (VarBuffer + Offset)) > Width) {
CHANGED->            if ( (StrSize ((CHAR16 *) (VarBuffer + Offset)) - sizeof(CHAR16)) > Width) {
                  return EFI_INVALID_PARAMETER;
            }
          }
          break;
Best Regards,
Wim Vervoorn

Eltan B.V.
Ambachtstraat 23
5481 SM Schijndel
The Netherlands

T : +31-(0)73-594 46 64
E : wvervoorn@eltan.com
W : http://www.eltan.com<http://www.eltan.com/>
"THIS MESSAGE CONTAINS CONFIDENTIAL INFORMATION. UNLESS YOU ARE THE INTENDED RECIPIENT OF THIS MESSAGE, ANY USE OF THIS MESSAGE IS STRICTLY PROHIBITED. IF YOU HAVE RECEIVED THIS MESSAGE IN ERROR, PLEASE IMMEDIATELY NOTIFY THE SENDER BY TELEPHONE +31-(0)73-5944664 OR REPLY EMAIL, AND IMMEDIATELY DELETE THIS MESSAGE AND ALL COPIES."


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-08-24  8:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-24  6:59 HiiValidateSettings issue with string item Wim Vervoorn
2017-08-24  8:35 ` Bi, Dandan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox