From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (NAM12-BN8-obe.outbound.protection.outlook.com [40.107.237.71]) by mx.groups.io with SMTP id smtpd.web08.12262.1618593163740822025 for ; Fri, 16 Apr 2021 10:12:44 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nvidia.com header.s=selector2 header.b=AG5XKwK2; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: nvidia.com, ip: 40.107.237.71, mailfrom: jbrasen@nvidia.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ND121mqxs82q0foieDF6eczAl7gjHc5TozfmU1ajWhMHttDcADy4LSrACwUBrQDLZiEAPKHm8EP3G8NqVV01ycVr2j369ePyve+mJ9eLKspQOWS7EhwrouaavuN/niyAmCXA5bbQAK5RpmOVBbIR0IbDpyyN3i9c/RWM3ZEp8iL2fGcmQqnAC0oi4R1kbZLrk6h/deNRX8Gu3uBLhZf4MEAR6QwC5Yh8i6S0DZL8C+uofbI+9Gs2mm4dJo/sgvkvLsO+H+RL18kvTdEJXz4vGXbFPaA0+arrvHeJ20yyfJxWh7kfrNK5AS/mWyoYrwTO4W5PNX/YXybrf3mQafHFSQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oYLRDSTAb1k2AODm2xBAp6Jaxm+05BSH4BLWRWX0Vwg=; b=W1fbJGd4jWk8kHg5wWOm71UuVa3YYxn8+mXG+P00Chhl0XOkhEzrfKLsmJSeuPSmpLkZ+2dFFk7pU9VddxdB49t/37ZNB/2PfHb1IsIQtOzJKhzQ/qPUdayFvKdT4cZLROGDINsisRSyb468g1jLxjEj4DzeaBI73z/DCjvcbbDAU34yUh0r33Khzn1d8Yarlo6h2A3n3X6u4HZtm7tysoivTUtwHJWJEQtF1ziZPZtJvQf3ygoWLpIKJynBGFhGDfKQMJfexJHFJAtWs04Ag2Qz8HyCNlHHG7nhhTTpsClu82UndrojX8UhqEe+anmXe3aWaVWoNjsU36GdnvgMnA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=oYLRDSTAb1k2AODm2xBAp6Jaxm+05BSH4BLWRWX0Vwg=; b=AG5XKwK2FHf8MGgQz5/8SM22Trh29KBxKP3GVqiRNbKHrhHaW8bnPV6WNw9pEbMjqXnLHadtzhAM8zn33wuss5FJf1NMKmR0o0rAtmWg3pSUfJBmUfLiRX0Uj/2O3gR78bC+8DeCZnsgun+ACTQm1/gD7LSIu75VfcndGavXr4W7cOpybEzNwimhxmys+ujb0wq/aL/L9Zc4psfxypRtT6UhTsAbwXxojE+JiI0+AsYI2PB/ikETb4B8Tfg4lyzUr4IlS+vMiWPrLB5H0gzAFa0U4aubIVuSxSqQ5ncZ9Fq560gLXMItOaMcsdpd35juMJ/MTx6BKuvLzVX4I+GCPA== Received: from DM6PR12MB3340.namprd12.prod.outlook.com (2603:10b6:5:3d::24) by DM6PR12MB4233.namprd12.prod.outlook.com (2603:10b6:5:210::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4042.19; Fri, 16 Apr 2021 17:12:42 +0000 Received: from DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::bcd6:8478:f472:a5eb]) by DM6PR12MB3340.namprd12.prod.outlook.com ([fe80::bcd6:8478:f472:a5eb%3]) with mapi id 15.20.4020.023; Fri, 16 Apr 2021 17:12:42 +0000 From: "Jeff Brasen" To: Bret Barkelew , "devel@edk2.groups.io" , "zhichao.gao@intel.com" , "afish@apple.com" , Laszlo Ersek CC: "Wang, Jian J" , "Wu, Hao A" , "Yao, Jiewen" , Liming Gao , "Ni, Ray" Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data Thread-Topic: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLib: Allow BMP with extra data Thread-Index: AQHXIAodQHGZzFyzO0CttJBNMkjws6qR120AgAErDYCAAD+V3IAANF6AgACPbgCAB8QRAIAEMVOAgBeK7Mo= Date: Fri, 16 Apr 2021 17:12:41 +0000 Message-ID: References: <70c26f78d461d1b8021462d3c3fe6eb717b19193.1616520420.git.jbrasen@nvidia.com> <7d5c8dd7-680c-98e6-b8a0-084704ca3021@redhat.com> <4834AF3E-A929-4911-AB8E-0665AB2033FB@apple.com>,, In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2021-04-01T17:26:07.0216087Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged; authentication-results: microsoft.com; dkim=none (message not signed) header.d=none;microsoft.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [2601:281:8100:1241:4d79:4253:57e4:4cb5] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 96f34f99-8116-4454-07d1-08d900fad850 x-ms-traffictypediagnostic: DM6PR12MB4233: x-microsoft-antispam-prvs: x-ms-exchange-transport-forked: True x-ms-oob-tlc-oobclassifiers: OLM:7691; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: A66iEzu0tzFbkCrfe8MziAON6e/gko8gTSvlVYncSdRWXEnLuoJ3m788QwSF4z7008WYWV6xL3V1jg20yR2SLeepkjWrJXCebMkC5HfxgQxxzX5Mpmkywr7m8+BDCmCzirltfzvPZIAfn1Le6xpKAL2H7urrdvtaWas0HnydzfYyia60BT3CCOxJcKHvSUjdzNHGek4GyiRTdWWXvTu0QQPh02Zq18MKFFe8WudP43sTPEhkQFjXpEc70aRqF6Ii/boleaWyhiuYH/qaoR9zkEclL8tRCbEE5/3ZDvDWfUm45mN4KCgS7UfKm/Mw50tXDFMSWxLg8BioaBse5ssuJQL8Nc8xCWMh9lU9xCYJCgd4q1f8g5iUAVGtlQwD6mw8uNMcMffugS/b414UVRlcVp8NyxilZUHqhs7bp1UorOiR8wHFpDnrC1lStFUDfBokCFbnk4gvosEkIM4cUmr0nSDXX1WeFD+/jx81oX2NiUqHZFAUyiO0BGIs92Cu30PNYiTeIzUu6P9sJDwO8FKkzPo9k2tHSiUnPZAYQqcYnelu4bgrHBF4VwXkxbbnmLdgHY9pGmQHQiJvdQ7QYEihUnS0jcFIG/fewwgPjJ/gRA2ZplH1dS+cazaz+wezD0GfoP2dn35P0L9L9ooXmhVJ265HcxVVoh8dM8m+gHSpyg9sZSOTUUWhsAkH2eC1aa4Q x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM6PR12MB3340.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(136003)(366004)(39860400002)(376002)(396003)(346002)(6506007)(110136005)(53546011)(9686003)(478600001)(5660300002)(8936002)(7696005)(83380400001)(45080400002)(76236003)(8676002)(99936003)(7416002)(19627405001)(52536014)(166002)(54906003)(2906002)(71200400001)(19627235002)(186003)(316002)(33656002)(966005)(66946007)(122000001)(38100700002)(66616009)(66556008)(64756008)(66446008)(66476007)(55016002)(4326008)(86362001)(76116006);DIR:OUT;SFP:1101; x-ms-exchange-antispam-messagedata: =?Windows-1252?Q?ZQnoo4TJjsZTyrFhvluJvr5dbDUfO/62ZP/7xf9Wxpn4BuGAVGTLUexe?= =?Windows-1252?Q?uhrqSyj7VnTgLuFFGgI1hCd1WUhtPXasoSZeanqBiSPVIwDRD6k/hQuz?= =?Windows-1252?Q?igPdNJNvW7QcSHLvBbevZAaVHtbPGwZS37FGxnBgBhNIMOAJ3JYLe4Ic?= =?Windows-1252?Q?fHC8fGTpg9Iv75O90JpTzBUJRLJhi/3l+RYjdY0BUnhpcywTAfJfNLZ9?= =?Windows-1252?Q?NQ0Qu1DZdEF1e+lR0jcaUa13NadpVO4dVBnWzrXK8Hq2xt42hQPsNVyv?= =?Windows-1252?Q?TX96VehKQaIuV53PvL98KqFoQ+amkwbjXB44c/q1u9Cf9ztq3P0+US1V?= =?Windows-1252?Q?pAz3QjyhPOnM8DFrr+exIs8m3zQKZHmCRyWzNzYfNXTcr9SJ0Nf0fKhu?= =?Windows-1252?Q?XE6Pc8dKm82EVLIQEVDwj8ZQIJF0uSY6Loj0heV3YbYvjqTJmHBLb9DX?= =?Windows-1252?Q?L+5fTrrHZtRddR4/iM5RbLK4XHE95u2HuhCTqOWpAi8yZKzseXGrF570?= =?Windows-1252?Q?cvsXXSvwOPxMtN0J1zwsq8r0SlRPb4MW6vpswvlb5JMTRHfHkOgS/Bi1?= =?Windows-1252?Q?MOo6/7fbpuxeeu524Dwi/BwcULJCDkwXB5Fby5eLPNRv5M6I/WJsMUn4?= =?Windows-1252?Q?G6TdYAhKWnz+ObvAAj2FH/Mx6S7t+Voz++a9nvfcQPNp4RkO1VTBX3il?= =?Windows-1252?Q?6x4nyUSiuCw0cAxmZCikrlyRpvX8D5BLrfoq14mvyQivNyX2mht4TA6a?= =?Windows-1252?Q?lPWKalwTuaso7N+vn3O81lpZdRtwKOpV6kDinqzs4Yyj1BpcqAMZea6R?= =?Windows-1252?Q?WbfasePODTjbNz8Tl05xKM/DaKgevJOy9hlmv/JvBzOkwgxCOaAuHnhS?= =?Windows-1252?Q?WFrFfEAOnKvYtJGcJMZ8EFtMJM9GYIXmUq9x4GHLTObYNnG3Ji0kZfcb?= =?Windows-1252?Q?UV8JfOuKOrbEcc7AKpDFNRCWlAg5CKaM0YvrSUSitBsqwESnbK3GIraF?= =?Windows-1252?Q?ho+CbORXHlUrK3/ZaKPvG4Tl/BADrbpIbiVOxbonzsHUod1fiCA6ZVnG?= =?Windows-1252?Q?GUAC+R/65XpdGZxra+ws0N5MVlKf9It+4Ac3mx2s95Apxkkl6UqbXYZ+?= =?Windows-1252?Q?QlT8MUOxLyqniUjAUhCX7TWst9tjB84F7DEK5acNcAyxbbNaEm9zkEx3?= =?Windows-1252?Q?6D1BKDMeqlUr5SGTdE9tXR1E2GOaHw3IQ/hGfurtWnrMrsbj9lO4WA4R?= =?Windows-1252?Q?zh0iw1tU0eC+CvcLnoqlNC1hH3B36nB5LSr2AJZrTtwZXH/E0floPpSA?= =?Windows-1252?Q?shJLB2cJqwSLT3gWBM69MRX7Wubu1VY83zvn8MuDOuQoGWDGFzxfji11?= =?Windows-1252?Q?An2lHizBEzcGkAamOssU9RaRLeJriZX3eiHZFPZT8XQt9fpAl9QyjTSM?= =?Windows-1252?Q?+hkl+PpI26nanumUK02eWfVIgMj2gUEwCtDV2NgutAkwyrnRWGQIHG4Y?= =?Windows-1252?Q?jje7k4hd?= MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3340.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 96f34f99-8116-4454-07d1-08d900fad850 X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Apr 2021 17:12:41.8944 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: TV6kmHLwDoHi+rv//XPJTQOhS3IPcR/ubjPpdbwwzZ8DJeq16wFQOdKVps75veCe4V0nyFDN0zPzUdoJwSC7hg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4233 X-Groupsio-MsgNum: 74213 Content-Language: en-US Content-Type: multipart/related; boundary="_004_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_"; type="multipart/alternative" --_004_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_ Content-Type: multipart/alternative; boundary="_000_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_" --_000_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable Sorry I was out for a bit so didn't get back to this thread for a bit. Who = should be on any additional security review? In TranslateBmpToGopBlt the Size is only used in DEBUG prints and this ver= ification check. Processing of the data uses the image structure data. In a= ddition BMP that have extra data between the color map (if present right af= ter bmp header) and the image data is allowed with an explicit comment. // // BMP file may has padding data between the bmp header section and th= e // bmp data section. // if (BmpHeader->ImageOffset - sizeof (BMP_IMAGE_HEADER) < sizeof (BMP_C= OLOR_MAP) * ColorMapNum) { return RETURN_UNSUPPORTED; } Thanks, Jeff ________________________________ From: Bret Barkelew Sent: Thursday, April 1, 2021 11:37 AM To: devel@edk2.groups.io ; zhichao.gao@intel.com ; afish@apple.com ; Laszlo Ersek Cc: Jeff Brasen ; Wang, Jian J = ; Wu, Hao A ; Yao, Jiewen ; Limin= g Gao ; Ni, Ray Subject: RE: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLi= b: Allow BMP with extra data External email: Use caution opening links or attachments I agree with the proposal for a deeper security review. I also would suggest that we can provide tooling with BaseTools to check a= nd/or correct the format of a BMP to match what the code expects (since the= re seems to be ambiguity in the spec/implementation). We=92ve got a validat= or in Mu and would be happy to put together some patches to at least get th= is started for the community to hammer on. - Bret From: Gao, Zhichao via groups.io Sent: Monday, March 29, 2021 6:35 PM To: devel@edk2.groups.io; afish@apple.com; Laszlo Ersek Cc: Jeff Brasen; Bret Barkelew; Wang, Jian J; Wu, Hao A<= mailto:hao.a.wu@intel.com>; Yao, Jiewen; Limin= g Gao; Ni, Ray Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLi= b: Allow BMP with extra data The patch would let the BMP file that with a bunch of data pass the check,= no matter the data is valid or not. Do we have other docs to descript whic= h data is allowed and valid? Correct the Cc mail address and invite more experts for security review. Thanks, Zhichao From: devel@edk2.groups.io On Behalf Of Andrew Fish= via groups.io Sent: Thursday, March 25, 2021 11:00 AM To: edk2-devel-groups-io ; Laszlo Ersek Cc: Jeff Brasen ; bret.barkelew@microsoft.com; Wang, J= ian J ; ao.a.wu@intel.com Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSupportLi= b: Allow BMP with extra data On Mar 24, 2021, at 11:26 AM, Laszlo Ersek > wrote: On 03/24/21 16:25, Jeff Brasen wrote: Some of the logo files we received for the group that makes our assets lik= e this (not sure what tool they were created with) look like they pad the B= MP size to 8 bytes. TranslateBmpToGopBlt: invalid BmpImage... BmpHeader->Size: 0xE1038 BmpHeader->ImageOffset: 0x36 BmpImageSize: 0xE1038 DataSize: 0xE1000 TranslateBmpToGopBlt: invalid BmpImage... BmpHeader->Size: 0x2A3038 BmpHeader->ImageOffset: 0x36 BmpImageSize: 0x2A3038 DataSize: 0x2A3000 TranslateBmpToGopBlt: invalid BmpImage... BmpHeader->Size: 0x5EEC38 BmpHeader->ImageOffset: 0x36 BmpImageSize: 0x5EEC38 DataSize: 0x5EEC00 So, each of these has 2 bytes of padding at the end of the file. We could = write a tool that would do the same size recalculation in order to update t= he size in the header and remove the two bytes but it seems that this is a = valid BMP file and it doesn't seem correct that UEFI is rejecting it. I can= update the commit message with more context if needed as well. If there's a spec describing the BMP format, Yes and there are various flavors as at some point I had some graphics giv= en to me in a format that did not work (I think it was BITMAPV4HEADER) :(. https://en.wikipedia.org/wiki/BMP_file_format#cite_note-DIBhelp-5 edk2 supports =91BM=92 and the BITMAPINFOHEADER DIB. I seem to remember DI= Bs are defined by the size. So =91BM' is a Microsoft Spec: https://docs.microsoft.com/en-us/previous-versions/ms969901(v=3Dmsdn.10)?r= edirectedfrom=3DMSDN The quote in that spec is: The file extension of a Windows DIB file is BMP. The file consists of a BI= TMAPFILEHEADER structure followed by the DIB itself. Unfortunately, because= the BITMAPFILEHEADER structure is never actually passed to the API, not ev= ery application that generates BMP files fills out the data structure caref= ully. To add to this confusion, the "proper" definition of the structure is= at odds with the documentation. Properly, the data structure contains the = following fields: The explanation of size field is: A DWORD that specifies the size of the file in bytes. The Microsoft Window= s Software Development Kit (SDK) documentation claims otherwise. To be on t= he safe side, many applications calculate their own sizes for reading in a = file. I would say that is not exactly a ringing endorsement from a spec point of= view on depending on that field. So it seems like that patch may be reason= able, but we should triple check it does not break any security related ass= umptions. Thanks, Andrew Fish and edk2 is needlessly strict, and the check can be relaxed without security risks, then I think a patch would be fair. Thanks Laszlo --_000_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable
Sorry I was out for a bit so didn't get back to this thread for a bit. Who= should be on any additional security review?

In TranslateBmpToGopBlt the Size is only used in DEBUG prints and thi= s verification check. Processing of the data uses the image structure data.= In addition BMP that have extra data between the color map (if present rig= ht after bmp header) and the image data is allowed with an explicit comment.

    //
    // BMP file may has padding data between the bmp header= section and the
    // bmp data section.
    //
    if (BmpHeader->ImageOffset - sizeof (BMP_IMAGE_HEADE= R) < sizeof (BMP_COLOR_MAP) * ColorMapNum) {
      return RETURN_UNSUPPORTED;
    }

Thanks,

Jeff

From: Bret Barkelew <Br= et.Barkelew@microsoft.com>
Sent: Thursday, April 1, 2021 11:37 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>; zhichao.gao@= intel.com <zhichao.gao@intel.com>; afish@apple.com <afish@apple.co= m>; Laszlo Ersek <lersek@redhat.com>
Cc: Jeff Brasen <jbrasen@nvidia.com>; Wang, Jian J <jian.j= .wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <= jiewen.yao@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Ni, = Ray <ray.ni@intel.com>
Subject: RE: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSu= pportLib: Allow BMP with extra data
 
External email: U= se caution opening links or attachments

I agree with the proposal for a deeper security r= eview.

 

I also would suggest that we can provide tooling = with BaseTools to check and/or correct the format of a BMP to match what th= e code expects (since there seems to be ambiguity in the spec/implementatio= n). We=92ve got a validator in Mu and would be happy to put together some patches to at least get this started = for the community to hammer on.

 

- Bret

 

From: <= a href=3D"mailto:zhichao.gao=3Dintel.com@groups.io">Gao, Zhichao via groups= .io
Sent: Monday, March 29, 2021 6:35 PM
To: devel@edk2.groups.io; afish@apple.com; Laszlo Ersek=
Cc: Jeff Brasen; Bret Barkelew; Wang, Jian J; Wu, Hao A; Yao, Jiewen; Liming Gao; Ni, Ray
Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSu= pportLib: Allow BMP with extra data

 

The patch would let the BMP file that with a bunc= h of data pass the check, no matter the data is valid or not. Do we have ot= her docs to descript which data is allowed and valid?

 

Correct the Cc mail address and invite more exper= ts for security review.

 

Thanks,

Zhichao

 

From: devel@edk2.groups.io <devel@edk2.= groups.io> On Behalf Of Andrew Fish via groups.io
Sent: Thursday, March 25, 2021 11:00 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Laszlo Ersek= <lersek@redhat.com>
Cc: Jeff Brasen <jbrasen@nvidia.com>; bret.barkelew@microsoft= .com; Wang, Jian J <jian.j.wang@intel.com>; ao.a.wu@intel.com
Subject: Re: [EXTERNAL] [edk2-devel] [PATCH 1/1] MdeModulePkg/BmpSu= pportLib: Allow BMP with extra data

 

 

 

On Mar 24, 2021, at 11:26 AM, Laszlo Ersek <lersek@redhat.com> wrote:

 

On 03/24/21 16:= 25, Jeff Brasen wrote:

Some of the logo files we received for the gro= up that makes our assets like this (not sure what tool they were created wi= th) look like they pad the BMP size to 8 bytes.

TranslateBmpToGopBlt: invalid BmpImage...
  BmpHeader->Size: 0xE1038
  BmpHeader->ImageOffset: 0x36
  BmpImageSize: 0xE1038
  DataSize: 0xE1000
TranslateBmpToGopBlt: invalid BmpImage...
  BmpHeader->Size: 0x2A3038
  BmpHeader->ImageOffset: 0x36
  BmpImageSize: 0x2A3038
  DataSize: 0x2A3000
TranslateBmpToGopBlt: invalid BmpImage...
  BmpHeader->Size: 0x5EEC38
  BmpHeader->ImageOffset: 0x36
  BmpImageSize: 0x5EEC38
  DataSize: 0x5EEC00

So, each of these has 2 bytes of padding at the end of the file. We could = write a tool that would do the same size recalculation in order to update t= he size in the header and remove the two bytes but it seems that this is a = valid BMP file and it doesn't seem correct that UEFI is rejecting it. I can update the commit message with m= ore context if needed as well.


If there's a spec describing the BMP format,

 

Yes and there are various flavors as at some poin= t I had some graphics given to me in a format that did not work (I think it= was BITMAPV4HEADER) :(. 

 

 

edk2 supports =91BM=92 and the BITMAPINFOHEA= DER DIB. I seem to remember DIBs are defined by the size. So =91BM' is a Mi= crosoft Spec:

 

The quote in that spec is:

 

The file exte= nsion of a Windows DIB file is BMP. The file consists of a <= span style=3D"font-size:12.0pt; font-family:"Segoe UI",sans-serif= ; color:#171717">BITMAPFILEHEADER structure followed by the DIB itself. Unfortunately, because the BITMAPFILEHEADER structure is never actually passed to the API, not every application that generates= BMP files fills out the data structure carefully. To add to this confusion= , the "proper" definition of the structure is at odds with the do= cumentation. Properly, the data structure contains the following fields:

 

The expl= anation of size field is:

A DWORD&= nbsp;that specifies the size of the file in bytes. The Microsoft Windows Software D= evelopment Kit (SDK) documentation claims otherwise. To be on the safe side= , many applications calculate their own sizes for reading in a file.=

 

I would say that is not exactly a ringing endorse= ment from a spec point of view on depending on that field. So it seems like= that patch may be reasonable, but we should triple check it does not break= any security related assumptions. 

 

Thanks,

 

Andrew Fish

 

and edk2 is nee= dlessly
strict, and the check can be relaxed without security risks, then I
think a patch would be fair.

Thanks
Laszlo



 

 

--_000_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_-- --_004_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_ Content-Type: image/png; name="F0EC9BD5722B471983D77D317B3A9240.png" Content-Description: F0EC9BD5722B471983D77D317B3A9240.png Content-Disposition: inline; filename="F0EC9BD5722B471983D77D317B3A9240.png"; size=140; creation-date="Thu, 01 Apr 2021 17:37:18 GMT"; modification-date="Thu, 01 Apr 2021 17:37:18 GMT" Content-ID: Content-Transfer-Encoding: base64 iVBORw0KGgoAAAANSUhEUgAAAsQAAAABCAYAAADZ77itAAAAAXNSR0IArs4c6QAAAARnQU1BAACx jwv8YQUAAAAJcEhZcwAADsMAAA7DAcdvqGQAAAAhSURBVEhL7cMBDQAACAMg+5cygQkeRoMIG9WT VVXVn7MHYi5moJeByLMAAAAASUVORK5CYII= --_004_DM6PR12MB3340CC1ABF2FD97FC50717D9CB4C9DM6PR12MB3340namp_--