1472676 - [Static Analysis] Possible null-dereference in dom/ipc/ContentChild.cpp

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect, P3)

Description

[Robert Bartlensky [:rbartlensky]]

infer reports the following errors:

dom/ipc/ContentChild.cpp:2263: error: NULL_DEREFERENCE
  pointer `chromeRegistry` last assigned on line 2261 could be null and is dereferenced at line 2263, column 3.
  2261.     nsChromeRegistryContent* chromeRegistry =
  2262.       static_cast<nsChromeRegistryContent*>(registrySvc.get());
  2263. >   chromeRegistry->RegisterRemoteChrome(packages, resources, overrides,
  2264.                                          locale, reset);
  2265.     static bool preloadDone = false;

dom/ipc/ContentChild.cpp:2281: error: NULL_DEREFERENCE
  pointer `chromeRegistry` last assigned on line 2277 could be null and is dereferenced at line 2281, column 7.
  2279.     switch (item.type()) {
  2280.       case ChromeRegistryItem::TChromePackage:
  2281. >       chromeRegistry->RegisterPackage(item.get_ChromePackage());
  2282.         break;
  2283.

dom/ipc/ContentChild.cpp:2285: error: NULL_DEREFERENCE
  pointer `chromeRegistry` last assigned on line 2277 could be null and is dereferenced at line 2285, column 7.
  2283.
  2284.       case ChromeRegistryItem::TOverrideMapping:
  2285. >       chromeRegistry->RegisterOverride(item.get_OverrideMapping());
  2286.         break;
  2287.

dom/ipc/ContentChild.cpp:2289: error: NULL_DEREFERENCE
  pointer `chromeRegistry` last assigned on line 2277 could be null and is dereferenced at line 2289, column 7.
  2287.
  2288.       case ChromeRegistryItem::TSubstitutionMapping:
  2289. >       chromeRegistry->RegisterSubstitution(item.get_SubstitutionMapping());
  2290.         break;
  2291.

These are all related to the fact that `nsChromeRegistry::GetService()` in all cases can return a `already_AddRefed<nsIChromeRegistry>` object whose member is nullptr. It seems that in all cases there are no null checks after extracting the pointer. Is it possible for the code to not return a nullptr, even though here (https://dxr.mozilla.org/mozilla-central/source/chrome/nsChromeRegistry.cpp?q=%2Bfunction%3AnsChromeRegistry%3A%3AGetService%28%29&redirect_type=single#135) there is a branch for that?

Comment 1

[Robert Bartlensky [:rbartlensky]]

infer also reports:

dom/ipc/ContentChild.cpp:778: error: NULL_DEREFERENCE
  pointer `*aTriggeringPrincipal` last assigned on line 778 could be null and is dereferenced at line 778, column 5.
  776.     if (!opener) {
  777.       nsCOMPtr<nsIPrincipal> nullPrincipal = NullPrincipal::CreateWithoutOriginAttributes();
  778. >     NS_ADDREF(*aTriggeringPrincipal = nullPrincipal);
  779.       return NS_OK;
  780.     }

Comment 2

[Andi [:andi]]

can we add a patch fixing these issues?

Comment 3

[Robert Bartlensky [:rbartlensky]]

Attachment: Bug 1472676: Fix NULL_DEREFERENCE errors

Review commit: https://reviewboard.mozilla.org/r/254760/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254760/

Comment 4

[Andrea Marchesini [:baku]]

Comment on attachment 8989769 [details]
Bug 1472676: Fix NULL_DEREFERENCE errors

https://reviewboard.mozilla.org/r/254760/#review261658

::: dom/ipc/ContentChild.cpp:2269
(Diff revision 1)
>                                   const bool& reset)
>  {
>    nsCOMPtr<nsIChromeRegistry> registrySvc = nsChromeRegistry::GetService();
>    nsChromeRegistryContent* chromeRegistry =
>      static_cast<nsChromeRegistryContent*>(registrySvc.get());
> +  if (!chromeRegistry)

if () {
  ..
}

::: dom/ipc/ContentChild.cpp:2282
(Diff revision 1)
>  ContentChild::RecvRegisterChromeItem(const ChromeRegistryItem& item)
>  {
>    nsCOMPtr<nsIChromeRegistry> registrySvc = nsChromeRegistry::GetService();
>    nsChromeRegistryContent* chromeRegistry =
>      static_cast<nsChromeRegistryContent*>(registrySvc.get());
> +  if (!chromeRegistry)

here as well.

Comment 5

[Robert Bartlensky [:rbartlensky]]

Attachment: Bug 1472676: Add curly brackets.

Review commit: https://reviewboard.mozilla.org/r/254762/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/254762/

Comment 6

[Andrea Marchesini [:baku]]

Comment on attachment 8989788 [details]
Bug 1472676: Add curly brackets.

https://reviewboard.mozilla.org/r/254762/#review261666

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9bd41553f056
Fix NULL_DEREFERENCE errors r=baku
https://hg.mozilla.org/integration/autoland/rev/61a41d0a2f7c
Add curly brackets. r=baku

Comment 8

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/9bd41553f056
https://hg.mozilla.org/mozilla-central/rev/61a41d0a2f7c