Undefined signed out-of-range shift in pkix_pl_object.c

NEW
Unassigned

Status

NSS
Libraries
5 years ago
2 years ago

People

(Reporter: decoder, Unassigned)

Tracking

({sec-want})

3.13.4
x86_64
Linux
sec-want

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [-fsanitize=shift])

(Reporter)

Description

5 years ago
A tool I'm currently testing is reporting undefined behavior from a fixed signed shift that is out of range here:

579         /* Initialize all object fields */
580         object->magicHeader = PKIX_MAGIC_HEADER;

where the defines resolve to this:

#define PKIX_MAGIC_HEADER           LL_INIT(0xFEEDC0FF, 0xEEFACADE)
#define LL_INIT(hi, lo)  ((hi ## L << 32) + lo ## L)

I guess the shift here is signed (long instead of unsigned long) and therefore the result is undefined when shifting by 32. It would be nice to fix this so I can continue using the tool.

Comment 1

5 years ago
do you have a proposed fix that makes it work with your tool? if yes, please attach a patch
(Reporter)

Comment 2

5 years ago
I made a quick fix like this in pkix_tools.h but this is surely not portable (also won't work on 32 bit i think):

// Warning, this code is not portable
#define ULL_INIT(hi, lo) ((hi ## UL << 32) + lo ## UL)

#define PKIX_MAGIC_HEADER           ULL_INIT(0xFEEDC0FF, 0xEEFACADE)
#define PKIX_MAGIC_HEADER_DESTROYED ULL_INIT(0xBAADF00D, 0xDEADBEEF)

The proper solution is likely to define the macros for LL_INIT based on unsigned variants, but I don't know where that would go.

Updated

5 years ago
Whiteboard: [-fsanitize=shift]

Updated

2 years ago
See Also: → bug 1180095
You need to log in before you can comment on or make changes to this bug.