[Bug 1926298] [NEW] Segfault added in the recent changes

[Launchpad Bug Tracker]

You have been subscribed to a public bug by Dan Bungert (dbungert):

[Impact]

On hirsute/impish, update-notifier can crash when attempting to run hooks.
This issue is the #2 crash on update-notifier in hirsute, presently (and
maybe some of the others).  I would call this a severe regression.

In the crash cases, cleanup steps are running on an uninitialized
variable.  The fix resolves this by ensuring the relevant variable is
initialized.

[Test Case]

Create a file 'bug'

'Name: bug
Priority: Low
Command: "/bin/true"
DisplayIf: /bin/true
Description: update notifier bug'

and copy it to /var/lib/update-notifier/user.d

[Where problems could occur]

This is not the most absolute minimal version of this fix.
If the minimal version of the fix is desired, I can provide that, which
should be a +2/-2 diff.  (I don't want to confuse the current proposal.)
The fix changeset being proposed includes other glib usage improvements
which sound really good from a maintainability perspective and may even
close other yet unknown bugs but add their own risk of regressions if
said maintainability was not done correctly.

The original idea was to address a problem where we offer to run a
command, and the user would click the button to do so, but then nothing
would happen because the command wasn't on the system.  If the parsing
of this command is invalid, then valid hooks may be filtered.

It's possible for the command to contain arguments, and to be quoted (or
not), or to not be the full command path.  Variables for such testing
include:
* rely on path lookup or fully qualified path supplied in command
* quoted or unquoted or mixed quotes (quote the command and args
  outside of the quotes)
* arguments or no
* presence or absence of actual command
See attached tarball for the resulting 20 tests (4 redundant tests pruned).
All the 'present' hooks are valid, 'absent' hooks should be skipped.

In an attempt to stave off further memory safety problems I've analyzed
this version with valgrind.  valgrind is able to observe several
problems in the unfixed version, whereas the fixed version only
complains about some items in glib that appear to be unrelated to this
change.

I've got some unit test code as well but haven't integrated that yet to
the update-notifier codebase.

[Original Bug Report Description]

The issue is new using
https://code.launchpad.net/~dbungert/update-notifier/+git/update-notifier/+merge/397367
, the previous revision didn't have the issue

* Create a file 'bug'

'Name: bug
Priority: Low
Command: "/bin/true"
DisplayIf: /bin/true
Description: update notifier bug'

and copy it to /var/lib/update-notifier/user.d

->  update-notifier segfaults

(gdb) bt
#0  0x00007ffff718c769 in __GI___libc_free (mem=0x5550000aa7f9) at malloc.c:3288
#1  0x00007ffff7365215 in g_strfreev (str_array=<optimized out>) at ../../../glib/gstrfuncs.c:2553
#2  g_strfreev (str_array=0x5555555d9e00) at ../../../glib/gstrfuncs.c:2546
#3  0x000055555555dd2d in hook_command_exists (cmd=0x555555702600 "\"/usr/bin/eog\"") at hooks.c:137
#4  0x000055555555fd6e in is_hook_relevant (hook_file=0x555555829e7b "apt-file.update-notifier")
    at hooks.c:717
#5  0x000055555555ffb1 in check_update_hooks (ta=0x5555556c9a20) at hooks.c:781
#6  0x0000555555560715 in hook_tray_icon_init (ta=0x5555556c9a20) at hooks.c:969
#7  0x000055555555cd77 in tray_icons_init (un=0x55555562c3e0,

It seems likely to be the error on top of the weekly report for hirsute
but that's missing a stacktrace

One comment in addition on the change there, was the env split + iterate
over pathdirs basically a re-implementation of
https://developer.gnome.org/glib/stable/glib-Miscellaneous-Utility-Functions.html#g-find-program-in-path
? If there is particular reason to do that I would recommend just using
the glib function instead (which might also fix the issue as a side
effect, I didn't check)

** Affects: update-notifier (Ubuntu)
     Importance: High
     Assignee: Dan Bungert (dbungert)
         Status: Fix Released

** Affects: update-notifier (Ubuntu Hirsute)
     Importance: High
         Status: Fix Committed


** Tags: fr-1314 patch regression-release verification-done-hirsute verification-needed
-- 
Segfault added in the recent changes
https://bugs.launchpad.net/bugs/1926298
You received this bug notification because you are a member of Ubuntu Sponsors Team, which is subscribed to the bug report.