Closed Bug 1180095 Opened 5 years ago Closed 5 years ago

UBSAN left shift of 4276994303 by 32 places cannot be represented in type 'long'


(NSS :: Libraries, defect, P2)



(firefox42 affected)

Tracking Status
firefox42 --- affected


(Reporter: tsmith, Assigned: wtc)


(Blocks 1 open bug)


(Keywords: sec-audit)


(3 files)

This was found by running the test suite with an Undefined Behavior Sanitizer (UBSAN) build.

pkix_pl_object.c:580:31: runtime error: left shift of 4276994303 by 32 places cannot be represented in type 'long'
#0 0x7f8dda82f7ff in PKIX_PL_Object_Alloc /home/user/Desktop/nss_ubsan/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_object.c:580 (discriminator 5)
#1 0x7f8dda81d2bd in PKIX_PL_HashTable_Create /home/user/Desktop/nss_ubsan/nss/lib/libpkix/pkix_pl_nss/system/pkix_pl_hashtable.c:149 (discriminator 1)
#2 0x7f8dda61e69f in PKIX_Initialize /home/user/Desktop/nss_ubsan/nss/lib/libpkix/pkix/top/pkix_lifecycle.c:91 (discriminator 1)
#3 0x7f8dda252ee3 in nss_Init /home/user/Desktop/nss_ubsan/nss/lib/nss/nssinit.c:688
#4 0x7f8dda253781 in NSS_Initialize /home/user/Desktop/nss_ubsan/nss/lib/nss/nssinit.c:813
#5 0x43e3c5 in certutil_main /home/user/Desktop/nss_ubsan/nss/cmd/certutil/certutil.c:2878
#6 0x435106 in main /home/user/Desktop/nss_ubsan/nss/cmd/certutil/certutil.c:3565
#7 0x7f8dd8e27ec4 in __libc_start_main /build/buildd/eglibc-2.19/csu/libc-start.c:287
#8 0x407e50 in _start ??:?
Mass cc to get some NSS eyes on these bugs.
Is there actual documentation for reproducing this with UBSAN? Also, can you provide more details about your setup - were you testing on Linux, Mac, Windows? 32-bit or 64-bit? contains line 580 at time of report. 

PKIX_MAGIC_HEADER is a #define,

LL_INIT comes from NSPR, which is defined at

The relevant #define should only be triggered if BYTES_PER_LONG is 8, at which point, it should be defined behaviour. Was there perhaps a misconfiguration when setting up UBSAN, such that ./configure reported PR_BYTES_PER_LONG as 8, but it was compiled at 4?
Thanks for the analysis, Ryan.

The stack trace in comment 0 shows several 64-bit addresses, so it looks
like a 64-bit build, and the pathnames of the source files show the build
was most likely done on a Linux platform (that uses eglibc-2.19).

Note that 4276994303 is 0xFEEDC0FF. The most significant bit is set. Since
'long' is a signed type, I wonder if UBSAN doesn't like us to shift a one
bit into the sign bit. I remember this is defined in the C standard.

Tyson: please change



    #define PKIX_MAGIC_HEADER           LL_INIT(0x7EEDC0FF, 0xEEFACADE)

and see if this fixes the UBSAN error. Thanks.

(Note the 'F' is changed to a '7'.)
OS: Unspecified → Linux
Hardware: Unspecified → x86_64
Sure thing. I will test it when I return from PTO next week.
Flags: needinfo?(twsmith)
Group: core-security → crypto-core-security
Tyson: Any update? :)
I guess I hit snooze on this one for too long. Sorry I'll have a look today :)
I tested with:

That seemed to make UBSsan happy, I did not the error.
Flags: needinfo?(twsmith)
Keywords: sec-audit
See Also: → 809813
The command that lead to this was:
pk12util -i ../stapling/ca.p12 -k ../ -w ../ -d ../stapling
Tyson: how do I do a UBSAN build of NSS? It would be more
efficient for the same person (either you or me) to do a
UBSAN build and experiment with various fixes.  Thanks.

In the experiment I proposed in comment 3, only the first
argument of LL_INIT() needs to be modified. The second
argument of LL_INIT() should still start with 0xE or 0xD.
Could you please try that experiment again?

There are three possible fixes.

1. What I proposed in comment 3. Note that the bug in
LL_INIT() still remains.

2. Fix the LL_INIT() macro. This is more work than the
other two options and is not worth our time in my opinion.

3. Consider the LL_INIT() macro as an obsolete feature
and change NSS to use PR_UINT64 instead:


I propose we do #3. But I'd appreciate if you could repeat
your experiment of #1 correctly.
Flags: needinfo?(nobody)
Flags: needinfo?(nobody) → needinfo?(twsmith)
Wan-Teh: I have tested both fixes as requested in comment 9. Both tests passed, the error was not reported by UBSan. 

Note: for #1 I used the the following values as requested.
Flags: needinfo?(twsmith)
This is the second fix I described in comment 9. I verified it
works. As the patch shows, it may break existing workarounds
for this bug and require undoing the workarounds.
The LL_INIT() macro was defined when not all compilers supported a
64-bit integer type. Now that all compilers support long long, we
can use the better PR_INT64() and PR_UINT64() macros.

When reviewing the changes for lib/util/dertime.c, please note that
the PRTime type is defined as PRInt64, and the code I modified was
added in this checkin:
Assignee: nobody → wtc
Attachment #8701207 - Flags: superreview?(kaie)
Attachment #8701207 - Flags: review?(franziskuskiefer)
Comment on attachment 8701207 [details] [diff] [review]
Use PR_INT64() or PR_UINT64() instead of LL_INIT()

Review of attachment 8701207 [details] [diff] [review]:

Attachment #8701207 - Flags: review?(franziskuskiefer) → review+
Comment on attachment 8701207 [details] [diff] [review]
Use PR_INT64() or PR_UINT64() instead of LL_INIT()

Review of attachment 8701207 [details] [diff] [review]:
Attachment #8701207 - Flags: checked-in+
This bug should not be marked as a security bug.
Closed: 5 years ago
OS: Linux → All
Priority: -- → P2
Hardware: x86_64 → All
Resolution: --- → FIXED
Target Milestone: --- → 3.22
agree, we should open up this bug
Flags: needinfo?(dveditz)
Group: crypto-core-security → core-security-release
Attachment #8701207 - Flags: superreview?(kaie)
Group: core-security-release
Flags: needinfo?(dveditz)
You need to log in before you can comment on or make changes to this bug.