1369543 - address some potentially unsafe snprintf uses in dom/

Status: RESOLVED WONTFIX

(Core Graveyard :: Plug-ins, defect, P3)

Description

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

snprintf returns the number of bytes it *would have* written when it runs out of buffer space. nsPluginHost::ParsePostBufferToFixHeaders attempts to handle this but doesn't quite get it right - the == needs to be a >= here: https://dxr.mozilla.org/mozilla-central/rev/bdb2387396b4a74dfefb7c983733eed3625e906a/dom/plugins/base/nsPluginHost.cpp#3545

In practice, it looks like the buffer will always be large enough, but we can better future-proof this.

Also, NetworkUtils::setInterfaceDns had a similar issue here: https://hg.mozilla.org/mozilla-central/annotate/deadb414ee23/dom/system/gonk/NetworkUtils.cpp#l1190
It's much less clear that this is safe. That code was removed in 55 by bug 1357323 and was b2g-related anyway, so it's unclear to me if there are any currently-supported projects that we would need to fix this in. I would appreciate some help in tracking this down.

Comment 1

[Gerry Chang [:gchang]]

Track 53-/54- as we've build 54 RC and there is not security level here. Feel free to nominate again if the security level is critical/high.

Comment 2

[Julien Cristau [:jcristau]]

tracking for 55.

Comment 3

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

This is a sec-low that doesn't need to be tracked for 55. We are already tracking several sec-high/sec-crits for 55. If a fix is ready and deemed low-risk, please nominate for uplift to Beta.

Comment 4

[Ryan VanderMeulen [:RyanVM]]

Not tracking for ESR52 either since it's sec-low.