From 004b8112380bb239a2b83524d890a697765f0d81 Mon Sep 17 00:00:00 2001 From: Ruiyu Ni Date: Thu, 16 Aug 2018 14:31:57 +0800 Subject: [PATCH] ShellPkg/edit: Fix heap access out-of-bounds The issue was found when heap guard is turned on. PrintLib somehow receives a non-null terminated string in var-arg. When the PrintLib implementation reads the string it keeps reading because no null-terminator is met, which triggers the page fault set by the heap guard. The issue is caused by a bug in FileBufferPrintLine(). When "edit" opens a binary file, in FileBufferPrintLine(), the Line->Buffer may start with \x00 \x00, but the Line->Size is larger than MainEditor.ScreenSize.Column, it causes the PrintLine is set to an empty string by below call: StrnCpyS ( PrintLine, BufLen/sizeof(CHAR16), Buffer, MIN(Limit, MainEditor.ScreenSize.Column) ); But since Limit (equals to Line->Size) is larger than MainEditor.ScreenSize.Column, below for-loop doesn't successfully set the whole PrintLine to all-empty-space. for (; Limit < MainEditor.ScreenSize.Column; Limit++) { PrintLine[Limit] = L' '; } So after the for-loop, PrintLine is still an empty string. Later in below call, the PrintLine2 is created based on PrintLine. ShellCopySearchAndReplace ( PrintLine, PrintLine2, BufLen * 2, L"%", L"^%", FALSE, FALSE ); But due to the implementation of ShellCopySearchAndReplace(), PrintLine2 is untouched and INVALID_PARAMETER is returned. Finally an uninitialized string is passed to ShellPrintEx() which causes the #PF exception. The fix is to reset Limit to StrLen(PrintLine) before for-loop. So that PrintLine can be converted from an empty string to a string containing all spaces. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Reviewed-by: Jaben Carsey Reviewed-by: Jian Wang --- ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c index 56ccd399b0..39a5afb53f 100644 --- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c +++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Edit/FileBuffer.c @@ -500,7 +500,7 @@ FileBufferPrintLine ( PrintLine = AllocatePool (BufLen); if (PrintLine != NULL) { StrnCpyS (PrintLine, BufLen/sizeof(CHAR16), Buffer, MIN(Limit, MainEditor.ScreenSize.Column)); - for (; Limit < MainEditor.ScreenSize.Column; Limit++) { + for (Limit = StrLen (PrintLine); Limit < MainEditor.ScreenSize.Column; Limit++) { PrintLine[Limit] = L' '; }