From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:4864:20::441; helo=mail-wr1-x441.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr1-x441.google.com (mail-wr1-x441.google.com [IPv6:2a00:1450:4864:20::441]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id AD44C2194D3B9 for ; Tue, 30 Oct 2018 02:27:15 -0700 (PDT) Received: by mail-wr1-x441.google.com with SMTP id 74-v6so3513735wrb.13 for ; Tue, 30 Oct 2018 02:27:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=MpDSFZDiBc5We0OY1sBcO5I4aSQisBXSPrVxni5BozA=; b=Ru5HO+hXURjX29cunLQejcuRK+uSKBa1XzqZnmHNWP7x70ISLfHtuaz1qdH4SGVjAS WZpv3V0iTh4zTqJltbN3ZiFfwHWZhRerpQ0BCWDi1/1bYcuUseJ+HCMRCUf2UNHzEKfZ FaAjQ0/T58OebJ6Rx4tWjyXFWzYYH51/bo+R4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=MpDSFZDiBc5We0OY1sBcO5I4aSQisBXSPrVxni5BozA=; b=f9WlJ3n/SoFGnsn7XWeDiORzwTH6Dze9KDOMzzWifOq9HFEf6sw+1KX2/SMWxzikE5 3y17NQYAcNfZf6KOBK2jAACKz7f7nKDzpicDjSGvY0QGWfaJKgq+s53sRjvdngRMA2fL wr572oZeY2iSs8HMaE65seJDod2DcfDsRUlMqe79l7YyLIyeXY4uNso8jwxzcUWnobej ZBYorrwmN3pWhXQ00kdq6Rt/+L2IfApE4pMgwg0U1OkjabSge+y2E26EiRKfNJb03aLQ kZEWsguFVbtnZ5lRToVmcd63SS6cUmzFrSDl3z2OvfGAljKLukaEV5hvkveeMmw4Ht3M YhRA== X-Gm-Message-State: AGRZ1gIes5Pd3+KG3PLdzbBzscfAurnDpoeyRibMSx37LvtZId7NwEQd EigsGWfgQQ2/icgnGDASzD1al/9nVcs= X-Google-Smtp-Source: AJdET5cbZTLuDjJrHoo3u4d0ZUEGrtyILe0tzn8ECSREDyVhpma7AxpU6r/k0aXoJXHoPMNnNq+Y5g== X-Received: by 2002:adf:f787:: with SMTP id q7-v6mr10617407wrp.9.1540891633660; Tue, 30 Oct 2018 02:27:13 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id j16-v6sm17107301wrq.89.2018.10.30.02.27.12 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 30 Oct 2018 02:27:12 -0700 (PDT) Date: Tue, 30 Oct 2018 09:27:11 +0000 From: Leif Lindholm To: Hao Wu Cc: edk2-devel@lists.01.org, Ruiyu Ni Message-ID: <20181030092710.d3jddppmpflswrpv@bivouac.eciton.net> References: <20181030012617.5040-1-hao.a.wu@intel.com> MIME-Version: 1.0 In-Reply-To: <20181030012617.5040-1-hao.a.wu@intel.com> User-Agent: NeoMutt/20170113 (1.7.2) Subject: Re: [PATCH v3 0/3] UdfDxe: Additional checks for ResolveSymlink() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Oct 2018 09:27:16 -0000 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Many thanks for the rework. For the series: Reviewed-by: Leif Lindholm On Tue, Oct 30, 2018 at 09:26:14AM +0800, Hao Wu wrote: > V3 changes: > > According to Leif's recommendation, split the original patch into 3 > seperate ones. > > Since there is no code changes compared with the V2 of the patch, I just > preserved the 'Reviewed-by' tags by Paulo and Star. > > V2 history: > > Refine type C check (refer to V1 history below) to eliminate the > unnecessary CopyMem() call. > > V1 history: > > The commit will add 3 types of checks for function ResolveSymlink(): > > A. Check for the value of 'Component Type' field within a Path Component > > According to the ECMA-167 standard (3rd Edition - June 1997), Section > 14.16.1.1, valid values are 1 to 5. All other values will be treated as a > corrupted volume. > > B. Check for the content pointed by 'File' > > Since content within 'File' is the output data for ResolveSymlink(). > Checks is added to ensure the content in 'File' is valid. Otherwise, > possible null pointer dereference issue will occur during the subsequent > usage of the data returned by ResolveSymlink(). > > C. Check for possible memory double free/use after free case > > For codes: > > if (CompareMem ((VOID *)&PreviousFile, (VOID *)Parent, > sizeof (UDF_FILE_INFO)) != 0) { > CleanupFileInformation (&PreviousFile); > } > > CopyMem ((VOID *)&PreviousFile, (VOID *)File, sizeof (UDF_FILE_INFO)); > > If the contents in 'PreviousFile' and 'File' are the same, call to > "CleanupFileInformation (&PreviousFile);" will free the buffers in 'File' > as well. This will lead to potential memory double free/use after free > issues. > > Cc: Leif Lindholm > Cc: Ruiyu Ni > > Hao Wu (3): > MdeModulePkg/UdfDxe: Check 'Component Type' within a Path Component > MdeModulePkg/UdfDxe: Content check for 'File' in ResolveSymlink() > MdeModulePkg/UdfDxe: Memory free/use after free in ResolveSymlink() > > MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 38 ++++++++++++++++++-- > 1 file changed, 35 insertions(+), 3 deletions(-) > > -- > 2.12.0.windows.1 >