From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Goldschmidt Date: Fri, 9 Nov 2018 22:25:10 +0100 Subject: [U-Boot] CVE-2018-18439, CVE-2018-18440 - U-Boot verified boot bypass vulnerabilities In-Reply-To: References: <20181106145150.GC10037@lambda.inversepath.com> <20181109094615.GC9586@lambda.inversepath.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 09.11.2018 11:24, Simon Goldschmidt wrote: > On Fri, Nov 9, 2018 at 10:46 AM Andrea Barisani > wrote: >> On Fri, Nov 09, 2018 at 07:11:36AM +0100, Simon Goldschmidt wrote: >>> On Fri, Nov 9, 2018 at 1:37 AM Fabio Estevam wrote: >>>> Hi Andrea, >>>> >>>> On Tue, Nov 6, 2018 at 12:57 PM Andrea Barisani >>>> wrote: >>>> >>>>> # load large file >>>>> => ext2load mmc 0 0x60000000 fitimage.itb >>>> Does this change work for you? >>>> http://dark-code.bulix.org/u6gw3b-499924 >>> My understanding was U-Boot text or stack could get overwritten which >>> leads to the loaded bytes being executed as code. >>> So you would have to check that the loaded range is within ram but not >>> within that reserved range of code or stack (or heap). >>> >> Exactly, merely checking RAM size is not sufficient. The specific memory >> layout would need to be accounted for which means understanding where the >> stack and heap are located, their direction of growth and to ensure that the >> loaded payload can never overwrite them along with all other U-Boot data >> segments. >> >> This is not easy given that the stack and heap size I think can only be >> guessed and not precisely limited, additionally board configurations have the >> ability to set arbitrary stack, relocation and load addresses which >> complicates things even further in understanding exactly how the memory >> layout is set. > It's not easy, but in my opinion, it should already be solved by the > code in 'boot_start_lmb' mentioned in my last mail. > This function includes arch and board callbacks that should be able to > return a safe memory range. I have a patched version running here with the above mentioned changes that should fix this issue. A few cleanups are missing before sending a patch though (changes are in fs.c and lmb.c only). Simon > > The only thing that cannot be controlled here is stack size, that's > true. The ARM port tries to solve this by getting the current stack > pointer and subtracting "4K to be safe". As far as I know, there are > no methods in U-Boot currently to ensure this is safe, though. And > depending on the RAM size, we could just subtract more. Personally, I > wouldn't mind subtracting some MBytes on my board. Actually using such > a stack would definively be another bug that needs fixing. > > But it seems a good start to use these functions to limit loading from fs, too. > > Simon > >>> Getting this reserved range is what 'boot_start_lmb' does (in >>> bootm.c). Maybe this code can be refactored and reused in fs.c to get >>> a valid range for loading? >>> >>> Additionally, your patch checks the loaded file's size without taking >>> the load address into account. So unless I read that wrong, your check >>> is only valid for 'addr == 0'. >>> Plus, the 'bytes' parameter should probably be a restriction to the >>> file's size when checking for a valid load range. >>> >>> Simon >> -- >> Andrea Barisani Head of Hardware Security | F-Secure >> Founder | Inverse Path >> >> https://www.f-secure.com https://inversepath.com >> 0x864C9B9E 0A76 074A 02CD E989 CE7F AC3F DA47 578E 864C 9B9E >> "Pluralitas non est ponenda sine necessitate"