mate-desktop / mate-panel

MATE panel
https://mate-desktop.org
GNU General Public License v2.0
184 stars 118 forks source link

weather_info_setup_tooltip heap-use-after-free #791

Closed Safari77 closed 6 years ago

Safari77 commented 6 years ago

Expected behaviour

no use-after-free 🤓🤞

Actual behaviour

    g_free (apparent);
    wind = weather_info_get_wind (info);
    if (strcmp (apparent, dgettext ("mate-applets-2.0", "Unknown")) != 0)

Fix is easy enough, move that g_free after apparent's last usage.

MATE general version

1.20.1

Package version

1.20.1

monsta commented 6 years ago

OMG. How did that work before...? It was there since GNOME 2.

monsta commented 6 years ago

Thanks for spotting it, https://github.com/mate-desktop/mate-panel/commit/7099408402dc41f39e32f4afa3e874e968e879bc should fix it.

muktupavels commented 6 years ago

I would say that it is not use-after-free, but copy-paste error! I think it should be wind there...

monsta commented 6 years ago

Hmm, wind is const gchar *...

muktupavels commented 6 years ago

it should be:

if (strcmp (wind, dgettext ("mate-applets-2.0", "Unknown")) != 0)

monsta commented 6 years ago

Ah, I see now. Not sure if wind can be "Unknown" too, but it makes more sense in this condition than apparent.

monsta commented 6 years ago

Ok, added https://github.com/mate-desktop/mate-panel/commit/3b9119b3bb941de61654d45eb693a85f48d56289 for that. Thanks.

Safari77 commented 6 years ago

What if weather_info* returned error codes (when something is unknown) so you don't have to do strcmp and dgettext and other obfuscated C coding contest things 🤔

monsta commented 6 years ago

Well, we have old API here, libgweather has something like that in gweather_info_get_value_wind function and others. Gotta update API or maybe try moving to libgweather.

raveit65 commented 6 years ago

We should really moved to libgweather soon.

monsta commented 6 years ago

Yeah, it's on my list for 1.22. It will be second try. The first one failed due to lots of differences in API.