Quite right - my copy and paste was a bit faster than it should have been. Here's an updated patch that properly differentiates SECTION1 and SECTION2 types.
SecurityPkg: fix Rsa2048Sha256GuidedSectionExtractLib issue causing section overruns and possible hangs due to bad output size calculation (updated) Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Eugene Cohen <[email protected]> --- .../DxeRsa2048Sha256GuidedSectionExtractLib.c | 4 ++-- .../PeiRsa2048Sha256GuidedSectionExtractLib.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c b/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c index 1f7a299..263eb9d 100644 --- a/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c +++ b/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c @@ -86,7 +86,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION2_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->DataOffset; + *OutputBufferSize = SECTION2_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } else { // // Check whether the input guid section is recognized. @@ -101,7 +101,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION *) InputSection)->DataOffset; + *OutputBufferSize = SECTION_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION_HEADER); } return EFI_SUCCESS; diff --git a/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c b/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c index e2a0fb6..252697a 100644 --- a/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c +++ b/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c @@ -84,7 +84,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION2_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->DataOffset; + *OutputBufferSize = SECTION2_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } else { // // Check whether the input guid section is recognized. @@ -99,7 +99,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION *) InputSection)->DataOffset; + *OutputBufferSize = SECTION_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION_HEADER); } return EFI_SUCCESS; -- -----Original Message----- From: Zhang, Chao B [mailto:[email protected]] Sent: Wednesday, October 14, 2015 9:32 PM To: Cohen, Eugene <[email protected]> Cc: [email protected] Subject: RE: [PATCH] SecurityPkg: fix Rsa2048Sha256GuidedSectionExtractLib issue causing section overruns and possible hangs due to bad output size calculation Cohen: I agree it is a mismatch between sectionGetInfo & SectionHandler. Your patch always minus sizeof (RSA_2048_SHA_256_SECTION2_HEADER). It is not correct for SECTION1. Thanks & Best regards Chao Zhang -----Original Message----- From: Cohen, Eugene [mailto:[email protected]] Sent: Wednesday, October 14, 2015 8:55 PM To: Zhang, Chao B Subject: FW: [PATCH] SecurityPkg: fix Rsa2048Sha256GuidedSectionExtractLib issue causing section overruns and possible hangs due to bad output size calculation In case you didn't see this on edk2-dev... -----Original Message----- From: edk2-devel [mailto:[email protected]] On Behalf Of Cohen, Eugene Sent: Tuesday, October 13, 2015 8:29 PM To: [email protected] Subject: [edk2] [PATCH] SecurityPkg: fix Rsa2048Sha256GuidedSectionExtractLib issue causing section overruns and possible hangs due to bad output size calculation Dear SecurityPkg maintainer, In Rsa2048Sha256GuidedSectionGetInfo the output buffer size is calculated like this: *OutputBufferSize = SECTION_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION *) InputSection)->DataOffset; but in Rsa2048Sha256GuidedSectionHandler the size is handled internally like this: OutputBufferSize = SECTION_SIZE (InputSection) - sizeof (RSA_2048_SHA_256_SECTION_HEADER); This means GetInfo is returning a buffer that is too large and causing the ProcessSection routine to process uninitialized data beyond the end of the extracted buffer which can cause hangs (if the memory that we overrun into is all zeros since the section processing does not advance) or worse (if non-zero data is interpreted as FFS section data and we go off in the weeds). The fix is to return the correct size in GetInfo so the section processing stops at the right point. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Eugene Cohen <[email protected]> SecurityPkg: fix Rsa2048Sha256GuidedSectionExtractLib issue causing section overruns and possible hangs due to bad output size calculation f7251883c495a3bd5de8cb5e6946ab653f6f0924 .../DxeRsa2048Sha256GuidedSectionExtractLib.c | 4 ++-- .../PeiRsa2048Sha256GuidedSectionExtractLib.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c b/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c index 1f7a299..593ce1f 100644 --- a/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c +++ b/SecurityPkg/Library/DxeRsa2048Sha256GuidedSectionExtractLib/DxeRsa2048Sha256GuidedSectionExtractLib.c @@ -86,7 +86,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION2_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->DataOffset; + *OutputBufferSize = SECTION2_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } else { // // Check whether the input guid section is recognized. @@ -101,7 +101,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION *) InputSection)->DataOffset; + *OutputBufferSize = SECTION_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } return EFI_SUCCESS; diff --git a/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c b/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c index e2a0fb6..95f9891 100644 --- a/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c +++ b/SecurityPkg/Library/PeiRsa2048Sha256GuidedSectionExtractLib/PeiRsa2048Sha256GuidedSectionExtractLib.c @@ -84,7 +84,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION2_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION2 *) InputSection)->DataOffset; + *OutputBufferSize = SECTION2_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } else { // // Check whether the input guid section is recognized. @@ -99,7 +99,7 @@ Rsa2048Sha256GuidedSectionGetInfo ( // *SectionAttribute = ((EFI_GUID_DEFINED_SECTION *) InputSection)->Attributes; *ScratchBufferSize = 0; - *OutputBufferSize = SECTION_SIZE (InputSection) - ((EFI_GUID_DEFINED_SECTION *) InputSection)->DataOffset; + *OutputBufferSize = SECTION_SIZE(InputSection) - sizeof (RSA_2048_SHA_256_SECTION2_HEADER); } return EFI_SUCCESS; _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
