1585170 - AddressSanitizer: heap-use-after-free [@ std::__atomic_base<unsigned long>::fetch_add] with WRITE of size 8 with race on SecurityInfo object

Status: RESOLVED DUPLICATE of bug 1674835

(Core :: Networking: HTTP, defect, P2)

Description

[Christian Holler (:decoder)]

I was experimenting with a modified fuzzing target on mozilla-central revision 04da00a61c6d (built with --enable-tests --enable-address-sanitizer --disable-jemalloc --enable-optimize=-O2 --enable-fuzzing --disable-debug) when I hit the attached crash.

In the new target, I create an HTTP channel as usual and call asyncOpen on it. However, unlike in the regular targets, the socket will simulate no data left at some point (by returning PR_WOULD_BLOCK) and then I call Cancel() on the channel. Afterwards, I create a second channel and repeat the request normally. Both calls are made with caching being active as well.

I was not able to reproduce the use-after-free so far, but I was able to capture a TSan trace that shows a thread race that looks related. I will add this as soon as the bug is filed. Note that the trace shows a problem with the FuzzySecurityInfo object. Normally, this object comes from NSS I believe, not sure if this makes the difference here.

Comment 1

[Christian Holler (:decoder)]

Attachment: Detailed Crash Information

Comment 2

[Christian Holler (:decoder)]

Attachment: Testcase

Comment 3

[Christian Holler (:decoder)]

Attachment: ThreadSanitizer Log

Comment 4

[Christian Holler (:decoder)]

Honza, if you could help me figure out if this is a legit bug or related to how FuzzySecurityInfo is done (maybe based on the TSan Log, since that seems most promising), that would be great as a start :)

Comment 5

[Honza Bambas (:mayhemer)]

The two racing places are:
https://hg.mozilla.org/mozilla-central/file/04da00a61c6d/netwerk/protocol/http/nsHttpChannel.cpp#l7680 (main)
https://hg.mozilla.org/mozilla-central/file/04da00a61c6d/netwerk/protocol/http/nsHttpTransaction.cpp#l1274 (socket)

The good thing is that this will likely not cause any real problems, because if we call Restart on the transaction we probably don't call OnStopRequest on the channel at the same time.

Still, this is a valid problem and should be fixed by a lock.

This is one of the cases that would be caught by bug 1582228 :)

Comment 6

[Daniel Veditz [:dveditz]]

sec-moderate because "will likely not cause any real problems" doesn't seem like a complete "this is not a security bug" exoneration.

Comment 7

[Kershaw Chang [:kershaw]]

This bug should be fixed by bug 1674835.