Open alex opened 2 weeks ago
This should probably copy to an integer type that's the same width as _Bool.
Do you know some integer type with width in 1 bit?
The C standard says: "While the number of bits in a _Bool object is at least CHAR_BIT, the width (number of sign and value bits) of a _Bool may be just 1 bit."
I don't see something wrong here.
There's no integer type with sub-byte width, however as you can see from that code, it parses values that are at least 1-byte (always 1-byte in reality, I assume) and treats 0s as 0s and anything else as true.
It's illegal in C to store a non 0 or 1 value in an _Bool
, hence the UBSAN warning.
You can see this in things like C compiler omitting any branch premised on an _Bool
having another value: https://godbolt.org/z/9hTjKMMjz
Take it another way. Why do you think, that p
value is arbitrary? This is a helper for Python's bool.
How you can call one with something else than 0 or 1 in *p?
Unless I have badly misread this code, p
comes from the argument to struct.unpack
, so its attacker controlled.
Simple reproducer:
~/p/cpython ❯❯❯ ./python.exe -c "import struct; print(struct.unpack('?', b'\x03'))"
python.exe(52009,0x1edf7f240) malloc: nano zone abandoned due to inability to reserve vm space.
Modules/_struct.c:502:28: runtime error: load of value 3, which is not a valid value for type 'bool'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior Modules/_struct.c:502:28 in
(True,)
There's no particular requirement that unpack be only used with pack-generated data, or valid data.
On Tue, Oct 8, 2024 at 1:16 PM Sergey B Kirpichev @.***> wrote:
(presumably packed by pack(format, ...))
— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you authored the thread.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
There's no particular requirement that unpack be only used with pack-generated data
I would rather say it's a soft requirement.
FYI, pr is ready: https://github.com/python/cpython/pull/125169
Thank you1
On Wed, Oct 9, 2024 at 12:11 AM Sergey B Kirpichev @.***> wrote:
There's no particular requirement that unpack be only used with pack-generated data
I would rather say it's rather a soft requirement.
FYI, pr is ready: #125169 https://github.com/python/cpython/pull/125169
— Reply to this email directly, view it on GitHub https://github.com/python/cpython/issues/125118#issuecomment-2401261886, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAGBGG2LRLEB7X6KWSOBTZ2SUIPAVCNFSM6AAAAABPSSWTPWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBRGI3DCOBYGY . You are receiving this because you authored the thread.Message ID: @.***>
-- All that is necessary for evil to succeed is for good people to do nothing.
I would rather say it's a soft requirement.
I don't think it is. struct.unpack
is used a lot to deal with data coming out of binary files that were written by some arbitrary other software, or from the network, or from a C library.
Bug report
Bug description:
https://github.com/python/cpython/blob/a5fc50994a3fae46d0c3d496c4e1d5e00548a1b8/Modules/_struct.c#L489-L491
Bool values are required to be either 0 or 1, but this memcpy will copy an arbitrary value to it. This produces UBSAN reports like:
(https://oss-fuzz.com/testcase-detail/5186406032080896)
This should probably copy to an integer type that's the same width as
_Bool
.CPython versions tested on:
CPython main branch
Operating systems tested on:
No response
Linked PRs