Hao Wu [mailto:hao.a...@intel.com] wrote:

]Sent: Tuesday, June 23, 2015 12:24 AM
]To: edk2-devel@lists.sourceforge.net
]Subject: [edk2] [PATCH 5/8] IntelFrameworkModulePkg BootMaint: Use safe string 
functions
]
]Contributed-under: TianoCore Contribution Agreement 1.0
]Signed-off-by: Hao Wu <hao.a...@intel.com>
]Reviewed-by: Jeff Fan <jeff....@intel.com>
]---
] .../Universal/BdsDxe/BootMaint/BootOption.c        | 60 +++++++++++++++++-----
] .../Universal/BdsDxe/BootMaint/FormGuid.h          | 16 ++++--
] .../Universal/BdsDxe/BootMaint/UpdatePage.c        |  9 +++-
] .../Universal/BdsDxe/BootMaint/Variable.c          | 10 ++--
] 4 files changed, 72 insertions(+), 23 deletions(-)
]
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]index 0a6a445..a55dd89 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/BootOption.c
]@@ -5,7 +5,7 @@
] 
]   Boot option manipulation
] 
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
] This program and the accompanying materials
] are licensed and made available under the terms and conditions of the BSD 
License
] which accompanies this distribution.  The full text of the license may be 
found at
]@@ -1012,7 +1012,11 @@ BOpt_GetBootOptions (
] 
]     NewLoadContext->Description = AllocateZeroPool 
(StrSize((UINT16*)LoadOptionPtr));
]     ASSERT (NewLoadContext->Description != NULL);
]-    StrCpy (NewLoadContext->Description, (UINT16*)LoadOptionPtr);
]+    StrCpyS (
]+      NewLoadContext->Description,
]+      StrSize((UINT16*)LoadOptionPtr) / sizeof (UINT16),
]+      (UINT16*)LoadOptionPtr
]+      );
]     
]     ASSERT (NewLoadContext->Description != NULL);
]     NewMenuEntry->DisplayString = NewLoadContext->Description;
]@@ -1089,6 +1093,7 @@ BOpt_AppendFileName (
] {
]   UINTN   Size1;
]   UINTN   Size2;
]+  UINTN   MaxLen;
]   CHAR16  *Str;
]   CHAR16  *TmpStr;
]   CHAR16  *Ptr;
]@@ -1096,18 +1101,31 @@ BOpt_AppendFileName (
] 
]   Size1 = StrSize (Str1);
]   Size2 = StrSize (Str2);
]-  Str   = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16));
]+  MaxLen = (Size1 + Size2 + sizeof (CHAR16)) / sizeof (CHAR16);
]+  Str   = AllocateZeroPool (MaxLen * sizeof (CHAR16));
]   ASSERT (Str != NULL);
] 
]-  TmpStr = AllocateZeroPool (Size1 + Size2 + sizeof (CHAR16)); 
]+  TmpStr = AllocateZeroPool (MaxLen * sizeof (CHAR16)); 
]   ASSERT (TmpStr != NULL);
] 
]-  StrCat (Str, Str1);
]+  StrCatS (
]+    Str,
]+    MaxLen,
]+    Str1
]+    );
]   if (!((*Str == '\\') && (*(Str + 1) == 0))) {
]-    StrCat (Str, L"\\");
]+    StrCatS (
]+      Str,
]+      MaxLen,
]+      L"\\"
]+      );
]   }
] 
]-  StrCat (Str, Str2);
]+  StrCatS (
]+    Str,
]+    MaxLen,
]+    Str2
]+    );
] 
]   Ptr       = Str;
]   LastSlash = Str;
]@@ -1120,11 +1138,19 @@ BOpt_AppendFileName (
]       //
] 
]       //
]-      // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of 
two strings 
]+      // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy 
of two strings 
]       // that overlap.
]       //
]-      StrCpy (TmpStr, Ptr + 3);
]-      StrCpy (LastSlash, TmpStr);
]+      StrCpyS (
]+        TmpStr,
]+        MaxLen,
]+        Ptr + 3
]+        );
]+      StrCpyS (
]+        LastSlash,
]+        MaxLen - (UINTN) (LastSlash - Str) / sizeof (CHAR16),

Does "/ sizeof (CHAR16)" belong here? I think the subtraction of
two CHAR16 pointers already gives a result in units of CHAR16.

]+        TmpStr
]+        );
]       Ptr = LastSlash;
]     } else if (*Ptr == '\\' && *(Ptr + 1) == '.' && *(Ptr + 2) == '\\') {
]       //
]@@ -1132,11 +1158,19 @@ BOpt_AppendFileName (
]       //
] 
]       //
]-      // Use TmpStr as a backup, as StrCpy in BaseLib does not handle copy of 
two strings 
]+      // Use TmpStr as a backup, as StrCpyS in BaseLib does not handle copy 
of two strings 
]       // that overlap.
]       //
]-      StrCpy (TmpStr, Ptr + 2);
]-      StrCpy (Ptr, TmpStr);
]+      StrCpyS (
]+        TmpStr,
]+        MaxLen,
]+        Ptr + 2
]+        );
]+      StrCpyS (
]+        Ptr,
]+        MaxLen - (UINTN) (Ptr - Str) / sizeof (CHAR16),

Does "/ sizeof (CHAR16)" belong here? I think the subtraction of
two CHAR16 pointers already gives a result in units of CHAR16.

]+        TmpStr
]+        );
]       Ptr = LastSlash;
]     } else if (*Ptr == '\\') {
]       LastSlash = Ptr;
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]index f2e1866..bf99999 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/FormGuid.h
]@@ -1,7 +1,7 @@
] /** @file
]   Formset guids, form id and VarStore data structure for Boot Maintenance 
Manager.
] 
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
] This program and the accompanying materials
] are licensed and made available under the terms and conditions of the BSD 
License
] which accompanies this distribution.  The full text of the license may be 
found at
]@@ -219,14 +219,20 @@ typedef struct {
] #define KEY_VALUE_SAVE_AND_EXIT_DRIVER         0x1002
] #define KEY_VALUE_NO_SAVE_AND_EXIT_DRIVER      0x1003
] 
]+//
]+// Description data and optional data size
]+//
]+#define DESCRIPTION_DATA_SIZE                  75
]+#define OPTIONAL_DATA_SIZE                     127
]+
] ///
] /// This is the data structure used by File Explorer formset
] ///
] typedef struct {
]-  UINT16  BootDescriptionData[75];
]-  UINT16  BootOptionalData[127];
]-  UINT16  DriverDescriptionData[75];
]-  UINT16  DriverOptionalData[127];
]+  UINT16  BootDescriptionData[DESCRIPTION_DATA_SIZE];
]+  UINT16  BootOptionalData[OPTIONAL_DATA_SIZE];
]+  UINT16  DriverDescriptionData[DESCRIPTION_DATA_SIZE];
]+  UINT16  DriverOptionalData[OPTIONAL_DATA_SIZE];
]   BOOLEAN BootOptionChanged;
]   BOOLEAN DriverOptionChanged;
]   UINT8   Active;
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]index 7d5861e..78380ad 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/UpdatePage.c
]@@ -1,7 +1,7 @@
] /** @file
] Dynamically update the pages.
] 
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
] This program and the accompanying materials
] are licensed and made available under the terms and conditions of the BSD 
License
] which accompanies this distribution.  The full text of the license may be 
found at
]@@ -830,7 +830,12 @@ UpdateConModePage (
]     //
]     UnicodeValueToString (ModeString, 0, Col, 0);
]     PStr = &ModeString[0];
]-    StrnCat (PStr, L" x ", StrLen(L" x ") + 1);
]+    StrnCatS (
]+      PStr,
]+      sizeof (ModeString) / sizeof (ModeString[0]),
]+      L" x ",
]+      sizeof (ModeString) / sizeof (ModeString[0]) - StrLen (PStr) - 1
]+      );
]     PStr = PStr + StrLen (PStr);
]     UnicodeValueToString (PStr , 0, Row, 0);
] 
]diff --git a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c 
b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]index e4299ff..616549e 100644
]--- a/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]+++ b/IntelFrameworkModulePkg/Universal/BdsDxe/BootMaint/Variable.c
]@@ -1,7 +1,7 @@
] /** @file
]   Variable operation that will be used by bootmaint
] 
]-Copyright (c) 2004 - 2014, Intel Corporation. All rights reserved.<BR>
]+Copyright (c) 2004 - 2015, Intel Corporation. All rights reserved.<BR>
] This program and the accompanying materials
] are licensed and made available under the terms and conditions of the BSD 
License
] which accompanies this distribution.  The full text of the license may be 
found at
]@@ -579,7 +579,7 @@ Var_UpdateDriverOption (
]     );
] 
]   if (*DescriptionData == 0x0000) {
]-    StrCpy (DescriptionData, DriverString);
]+    StrCpyS (DescriptionData, DESCRIPTION_DATA_SIZE, DriverString);
]   }
] 
]   BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize (DescriptionData);
]@@ -763,7 +763,11 @@ Var_UpdateBootOption (
]   UnicodeSPrint (BootString, sizeof (BootString), L"Boot%04x", Index);
] 
]   if (NvRamMap->BootDescriptionData[0] == 0x0000) {
]-    StrCpy (NvRamMap->BootDescriptionData, BootString);
]+    StrCpyS (
]+      NvRamMap->BootDescriptionData,
]+      sizeof (NvRamMap->BootDescriptionData) / sizeof 
(NvRamMap->BootDescriptionData[0]),
]+      BootString
]+      );
]   }
] 
]   BufferSize = sizeof (UINT32) + sizeof (UINT16) + StrSize 
(NvRamMap->BootDescriptionData);
]-- 
]1.9.5.msysgit.0



------------------------------------------------------------------------------
Monitor 25 network devices or servers for free with OpManager!
OpManager is web-based network management software that monitors 
network devices and physical & virtual servers, alerts via email & sms 
for fault. Monitor 25 devices for free with no restriction. Download now
http://ad.doubleclick.net/ddm/clk/292181274;119417398;o
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to