I reviewed fwupdate version 0.5-1 as checked into xenial. This should not be considered a full security audit but rather a quick gauge of maintainability.
- fwupdate helps Linux hosts install and run UEFI-based updates - Build-Depends: debhelper, pkg-config, libpopt-dev, libefivar-dev, lsb-release, gnu-efi, elfutils - Does not itself do networking or cryptography - Does not itself daemonize - pre/post inst/rm looks to properly undo EFI installed bits on uninstall - No dbus services - No setuid files - fwupdate executable in the path - No sudo fragments - No udev rules - No tests run during build or autopkgtest - No cronjobs - Relatively clean build logs - No subprocesses spawned - Memory management is very unusual (e.g. onstack() function) but looks disciplined and careful - File IO sets up EFI boot environments, looks careful - Logging looks careful - One environment variable, looked careful, best to say it should only be set by an admin: LIBFWUP_ESRT_DIR - Privileged operations are privileged by nature of writing to the EFI boot space, variables - No cryptography - No networking - Temporary file handling looked careful - No WebKit - Clean cppcheck, some shellcheck issues - No PolicyKit This code is clean and well-written. I had reviewed an earlier version of fwupdate and reported some issues and had some suggestions; the fwupdate team took those suggestions seriously and addressed everything with aplomb and style. (I didn't investigate each change individually but the end result is clear, the package is clean and a pleasure to read.) Security team ACK for promoting fwupdate to main. Here's the notes I took while reviewing this package, along with a lintian error and shellcheck output: - fwup_set_up_update() lots of work between final fputc() and fclose() -- does this need fflush(fout); fsync(outfd); before the work? - get_fd_and_media_path() asprintf error path should not use "goto out;" because the fullpath variable is undefined (by asprintf(3)). Lintian error: E: libfwup0: postinst-must-call-ldconfig usr/lib/x86_64-linux-gnu/libfwup.so.0.5 shellcheck output: ./linux/cleanup.in:10:20: note: Double quote to prevent globbing and word splitting. [SC2086] ./linux/bash-completion:8:40: note: Double quote to prevent globbing and word splitting. [SC2086] ./linux/bash-completion:14:52: warning: This { is literal. Check expression (missing ;/\n?) or quote it. [SC1083] ./linux/bash-completion:14:66: warning: This } is literal. Check expression (missing ;/\n?) or quote it. [SC1083] ./linux/bash-completion:14:77: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/fwupdate.postinst:28:25: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/fwupdate.postinst:29:24: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/fwupdate.postinst:34:25: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/fwupdate.postinst:35:28: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/fwupdate.postrm:25:29: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:11:34: note: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead. [SC2002] ./debian/scripts/install:15:15: warning: For loops over find output are fragile. Use find -exec or a while read loop. [SC2044] ./debian/scripts/install:22:22: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:32:25: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:34:17: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:35:20: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:37:14: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/scripts/install:37:23: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/libfwup0.install:3:1: note: bindir appears unused. Verify it or export it. [SC2034] ./debian/libfwup0.install:5:1: note: includedir appears unused. Verify it or export it. [SC2034] ./debian/libfwup0.install:7:1: note: mandir appears unused. Verify it or export it. [SC2034] ./debian/libfwup0.install:9:6: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/libfwup-dev.install:3:1: note: bindir appears unused. Verify it or export it. [SC2034] ./debian/libfwup-dev.install:7:1: note: mandir appears unused. Verify it or export it. [SC2034] ./debian/libfwup-dev.install:11:6: note: Double quote to prevent globbing and word splitting. [SC2086] ./debian/libfwup-dev.install:12:6: note: Double quote to prevent globbing and word splitting. [SC2086] Thanks ** Changed in: fwupdate (Ubuntu) Assignee: Seth Arnold (seth-arnold) => (unassigned) -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1508926 Title: [MIR] fwupdate To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/fwupdate/+bug/1508926/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs