19 ms·
CVE-2023-40547 – avoid incorrectly trusting HTTP headers
- guytv 3y agoUsing the 'goto' keyword brings back lots of memories after all these years.
- tsimionescu 3y agoAny idea why this code is using two measures of the body at all? Per the RFC, in HTTP/1.1, the Content-Length is the authoritative information for how long the HTTP request/response body is. Any data on the wire past the content length is part of another message, by definition. Conversely, if Content-Length is larger then rx_message.BodyLength, that means that we didn't yet receive the whole message, so we should either wait more, or issue a timeout error. Either way, rx_message.BodyLength is a bogus value if it's not guaranteed to be equal to Content-Length. On the other hand, if you want to be more permissive, then why look at the Content-Length header at all? Just use rx_message.BodyLength as the buffer size and try to interpret all the data on the wire as the received message. The current version of the code is needlessly complex, and that is how bugs like this make it in.
- oefrha 3y agoLooking at the commit in isolation can be somewhat misleading. If you look at the surrounding code https://github.com/rhboot/shim/blob/0226b56513b2b8bd5fd281bce77c40c9bf07c66d/httpboot.c#L572-L638 https://github.com/rhboot/shim/blob/0226b56513b2b8bd5fd281bc..., it's receiving chunks of data in a loop, checking to make sure new data does not exceed the buffer capacity (determined by Content-Length) each time. However, previously it wasn't checking that on the very first read outside the loop, hence the bug. I don't see any final check on downloaded == *buf_size (Content-Length) though. Violating that could indicate premature connection close.
- tsimionescu 3y agoOh, thanks for that extra context. I did look a bit further down, but didn't see the loop... This makes much more sense. Looking at the loop though, I think it does ensure that downloaded == *buf_size: the loop continues while downloaded < *buf_size, and there is a different check for not overflowing. So, overall, downloaded must be exactly equal to *buf_size to exit the loop without an error. So, this seems OK overall.
- oefrha 3y agoOh yeah, thought I saw a break, apparently not.
- ImJamal 3y agoIs this only a problem if you are doing some sort of network boot or do boot loaders connect to a network for another reason?
- nailer 3y agoNetwork boot. If you have physical (or lights out) access to a box you can own it though.
- nightowl_games 3y agoWOW It seems so obvious!
- mlhpdx 3y agoIndeed. The header reads to me as “the server says this content length will be…” so I have always taken precautions with it. Servers can be wrong as well as malicious.
- zadokshi 3y agoYep I’m surprised this made it through even basic code review. It makes it seem like this code was written by a junior dev without any code review. Accidents happen; it’s human and understandable; but wowzer this one seems like a beginner level mistake.
- rany_ 3y agoBut do these builds of shim include httpboot? AFAIK shim is just there to execute another EFI binary on the disk. I don't think I've ever seen the netboot functionality of shim get used.
- Arnavion 3y agoshim is an EFI bootloader commonly used by Linux distros that want to enable Secure Boot. Distros want an easy path to enabling Secure Boot using the default preloaded Microsoft signing key, rather than require users to enroll their own key. But Microsoft refuses to sign GPL bootloaders like grub in general, so shim was made to be able to be signed by the Microsoft key. (shim then implements its own separate signature verification of what it boots, based on a different key called the Machine Owner Key, MOK.) When you tell shim what EFI binary to boot, you can specify it as an HTTP URL. If you do this and the HTTP server is malicious, it can trigger an out-of-bounds write. However, usually you use it to boot a local second-stage bootloader like grub (hence the name "shim"), so it's unlikely to be a problem for most installs. Regardless, Secure Boot was built from the start to allow previously-signed binaries to be revoked via the DBX list - a list of signatures that can be loaded into the UEFI to make it reject binaries even if they have a valid signature. When (If) the signatures of old shim binaries with this bug are added to the list, you can update the list in your own machines. Updates may be distributed as capsule updates (via LVFS etc), or if you manage your SB keys and vars you can download and enroll the list from https://uefi.org/revocationlistfile https://uefi.org/revocationlistfile
- Dalewyn 3y ago>However, usually you use it to boot a local second-stage bootloader like grub (hence the name "shim") Would Windows Boot Manager not serve that purpose also? I have WBM chainloading into GRUB on one of my older BIOS machines, but I never got around to doing that on a UEFI machine so I'm not aware of any showstoppers.
- earthling8118 3y agoWindows is a showstopper itself. It might play nicely sometimes but it isn't a workable solution. I can no longer ever try dual booting because it doesn't respect anything but itself. It'll just decide to get rid of other bootloaders at its own discretion.
- Dalewyn 3y ago
- 1970-01-01 3y agoThe S in HTTP is to be used when important things are happening. This includes booting your device. HTTPS headers have always been encrypted. Still a good catch.
- oconnore 3y agoHTTPS is irrelevant here. You can send incorrect headers in either.
- tsimionescu 3y agoThe flaw here is only exploitable by (1) a malicious server, which could anyway just send a malicious binary, no need for header shenanigans, or (2) a MITM. Case 1 is moot, case 2 would be prevented by properly implemented HTTPS. On the other hand, I don't think it's practical to actually implement HTTPS properly in UEFI, since you'd have to constantly update the trust store, and you'd have to have actual internet access to be be able to check the certificate revocation lists (otherwise, you are vulnerable to surreptitious malicious activity from otherwise trusted CAs).
- oconnore 3y ago> Case 1 is moot, case 2 would be prevented by properly implemented HTTPS. That's not true. It's significantly easier to ensure the security of an offline signing key than it is to ensure that an arbitrary HTTPS server avoids ever becoming compromised.
- 1970-01-01 3y agoMITM scenarios say it isn't :P
- 38 3y agoyou cant MITM HTTPS unless you're are doing it to yourself for testing.
- 3y ago
- stonogo 3y agoI have to say this is definitely a bug and I'm glad it's fixed ... but what kind of psycho boots their machine from an untrusted host? If the attacker controls the http service tightly enough to send malicious headers, avoiding this overflow is the least of your problems, since they have compromised your certs and also can just send a compliant payload with more malware on it. Bug? absolutely. Critical? not so sure.
- deleted 3y ago[deleted]
- hackernudes 3y agoPerhaps it could be used to break in to some locked devices.
- deleted 3y ago[deleted]
- TheSoftwareGuy 3y agoThe only good form of defense, is defense in depth, or forming layers of defense. This bug puts a hole in one of those layers
- JackSlateur 3y agoWell, yes, but actually, no
- AshamedCaptain 3y agoThe same kinds of psycho who promote Secure Boot. They _really_ believe in a security strategy which _requires_ everything that could be potentially be compromised to not be signed by Secure Boot. The moment you have something signed which has a vulnerability, you can use it to decrypt everyone's SecureBoot+TPM encrypted hard drives. Why that ever was considered a valid security approach is over me, as there is an entire gallery of vulnerabilities like this one. It also completely ignores the elephant in the room called Windows. Example of this type of thinking: https://lkml.org/lkml/2018/4/3/767 https://lkml.org/lkml/2018/4/3/767 Note that despite Linus' reticences, in a lot of distros integrity mode is indeed enabled when you boot with Secure Boot on. Likely because of MS politics and distros wanting an MS UEFI signature being forced to "go through the hoops" as explained in that thread. As a result enabling Secure Boot usually cripples your distro, preventing you from e.g. hibernating.
- deleted 3y ago[deleted]
- pizzalife 3y ago> Content-Lenght
- hypeatei 3y agoVery embarrassing and unacceptable. /s
- frankjr 3y ago> Critical bug that exists in every Linux boot loader signed in the past decade Huh? This is a bug in a single bootloader. It has nothing to do with other bootloaders. And while on the subject, consider whether you actually need a bootloader at all.
- ixtli 3y agoAlso worth noting the commit which introduced this was made 8 years ago so not quite a decade.
- merb 3y agouf. I think a lot of frameworks had these issues. but nowadays its either rejection of the request or handlded like chunked encoding
- silisili 3y agoMaybe I'm not well versed enough, but I thought most http clients read at most content-length specified, and consider it an error if read bytes < content-length.
- Arnavion 3y agoThe HTTP client is provided as an EFI driver by the UEFI. AFAICT the UEFI spec doesn't specifically say what the behavior should be if the content-length header doesn't match the response body length, so it might very well be possible that some implementations just make `connection:close` requests and don't check the content length. The vulnerability was reported by MSRC and none of the text about the CVE mentions an actual exploit. It might be revealed later, or it might just be theoretical.
- tsimionescu 3y agoThe HTTP spec does say though: in HTTP/1.1, the HTTP body length is the value of the content-length header. Anything else coming on the wire is part of the next HTTP request/response (which may of course be invalid). Reading everything from the stream until the connection is closed is HTTP/1.0 behavior. Of course, if the UEFI HTTP client is not correctly implementing the spec, it's up to shim to defend itself, so I'm not saying this change is wrong or unnecessary.
- Arch-TK 3y agoFor anyone wondering "Why would you boot from an untrusted/compromised server?" or "Isn't this moot? If the server is compromised, it can just send a malicious binary." the short answer is: The binary that the shim ends up booting has to be signed by the MOK. So it should continue to provide the same security guarantees whether you're booting from within a compromised network, over HTTP, or if you're booting from a compromised server (even if it's over HTTPS). It's also important to note that secure boot doesn't prevent downgrades, so the fact that a compromised server could be utilised in a downgrade attack is not relevant. You would need to implement downgrade attack prevention in a more robust way regardless. That being said, I don't know _why_ the shim needs to support http boot (after all, nothing stops this from being implemented as second local EFI binary which handles it and is signed by the MOK), aside from maybe that it was thought to be a relatively simple feature to implement.
- barbazoo 3y agoCan someone explain to me please why reading less than the actual length of the body is dangerous? I’d have expected the opposite to be case.
- dwattttt 3y agoFrom the description, it allocates a buffer based on Content-Length, but copies the size of the buffer it received, thus writing out of bounds of the allocation.
- lcnPylGDnU4H9OF 3y ago> When retrieving files via HTTP or related protocols, shim attempts to allocate a buffer to store the received data. Unfortunately, this means getting the size from an HTTP header, which can be manipulated to specify a size that's smaller than the received data. In this case, the code accidentally uses the header for the allocation but the protocol metadata to copy it from the rx buffer, resulting in an out-of-bounds write.
- pwdisswordfishc 3y agoExcept that Content-length is not the length of the actual body, it is the length after Content-encoding...
- Joker_vD 3y ago"HTTP/1.1 is a delightfully simple protocol, if you ignore most of it".
- hiddencost 3y agoThe typo fix warms my cold dead corporate heart.
- userbinator 3y agoFuck "secure" boot. The fact that it's given one megacorp a monopoly over "security" should make you think twice about who actually benefits. The apologists will always say "but you can turn it off" "use your own keys" etc., yet M$ lost an antitrust suit over merely having IE bundled with the OS. It's sad to see the once-open PC platform slowly turn into a walled garden. I pray that there are more "bugs" which remain undiscovered.
- Andrex 3y agoPXE/HTTP boot just sound like bad ideas with worse implementations. My own smell test just screams "security nightmare."