[Bug 1508926] Re: [MIR] fwupdate

[Steve Langasek]

fwupdate-signed promoted to main.

** Changed in: fwupdate-signed (Ubuntu)
   Status: Fix Committed => Fix Released

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Steve Langasek]

Override component to main
fwupdate 0.5-2 in xenial: universe/misc -> main
fwupdate 0.5-2 in xenial amd64: universe/admin/optional/100% -> main
fwupdate 0.5-2 in xenial arm64: universe/admin/optional/100% -> main
fwupdate 0.5-2 in xenial armhf: universe/admin/optional/100% -> main
fwupdate 0.5-2 in xenial i386: universe/admin/optional/100% -> main
libfwup-dev 0.5-2 in xenial amd64: universe/libdevel/optional/100% -> main
libfwup-dev 0.5-2 in xenial arm64: universe/libdevel/optional/100% -> main
libfwup-dev 0.5-2 in xenial armhf: universe/libdevel/optional/100% -> main
libfwup-dev 0.5-2 in xenial i386: universe/libdevel/optional/100% -> main
libfwup0 0.5-2 in xenial amd64: universe/libs/optional/100% -> main
libfwup0 0.5-2 in xenial arm64: universe/libs/optional/100% -> main
libfwup0 0.5-2 in xenial armhf: universe/libs/optional/100% -> main
libfwup0 0.5-2 in xenial i386: universe/libs/optional/100% -> main
13 publications overridden.


** Changed in: fwupdate (Ubuntu)
   Status: Fix Committed => Fix Released

-- 
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

Re: [Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

Ah OK, thanks that makes sense.

On Fri, Mar 11, 2016 at 9:40 PM Michael Terry 
wrote:

> Yeah, it doesn't get pulled into main until something in main depends on
> it.  So in this case, we're probably waiting for fwupd's MIR (bug
> 1536871).  And then for gnome-software to depend on fwupd and pull it
> all in.
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> 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
>

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Michael Terry]

Yeah, it doesn't get pulled into main until something in main depends on
it.  So in this case, we're probably waiting for fwupd's MIR (bug
1536871).  And then for gnome-software to depend on fwupd and pull it
all in.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

@mterry,

I noticed this is still actually in universe.  Is there something else
that needs to be done for it to finish the move to main?

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

Yeah it looks like it was the -signed package.  That upload caused the
proposed migration to happen.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

Once the MIR for fwupd
(https://bugs.launchpad.net/ubuntu/+source/fwupd/+bug/1536871) is done
I'm expecting that  firmware support will be turned on in gnome-software
(https://bugs.launchpad.net/ubuntu/+source/gnome-software/+bug/1544376).

In my mind either gnome-software should recommend the signed version, or
the desktop seeds should.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Michael Terry]

Looked at the -signed version too, it's fine.

What pulls the -signed version in?  Does it need to be seeded or does
something depend on it?

** Also affects: fwupdate-signed (Ubuntu)
   Importance: Undecided
   Status: New

** Changed in: fwupdate-signed (Ubuntu)
   Status: New => Fix Committed

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

I think it might be be because 0.5-2 synced from Debian and fwupdate-
signed needed to be redone.  I just bumped fwupdate-signed version to
1.4 to see if that does the trick on migration.

Does fwupdate-signed need an MIR too?  Or since it's built from fwupdate
and just signing the binary it's implicitly promoted too?

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Michael Terry]

What's stopping fwupdate from migrating from proposed?  update_excuses
has it as a valid candidate.

Further fixes are great, but not blockers for promotion.  With Seth's
ACK and a look myself at the packaging, fwupdate seems fine to me.

** Changed in: fwupdate (Ubuntu)
   Status: In Progress => Fix Committed

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

Thanks Seth.
I've submitted upstream a fix for get_fd_and_media_path and the various 
shellcheck fixes.  I expect they should be merged for the next fwupdate release.
https://github.com/rhinstaller/fwupdate/pull/45/commits

I'm unsure if fflush(fout) is needed between fputc() and fclose().  I think 
that's better answered by upstream.
https://github.com/rhinstaller/fwupdate/issues/46

The shellcheck fixes in debian/ I've commited to debian-next (which will be 
uploaded on the next fwupdate release).
http://anonscm.debian.org/cgit/uefi/fwupdate.git/log/?h=debian-next


Let me know if anything called out is a blocker for promotion, otherwise expect 
fixes for all at next fwupdate release.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Seth Arnold]

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 

[Bug 1508926] Re: [MIR] fwupdate

[Tyler Hicks]

** Changed in: fwupdate (Ubuntu)
   Status: Incomplete => In Progress

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

Just wanted to check in on this security review for MIR.  Seth is this
still in your queue?

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Robert Ancell]

Required for fwupd MIR (bug 1536871)

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Robert Ancell]

** Tags added: gnome-software-ubuntu

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Jackson Doak]

The 0.5 release is in ubuntu now, fixing the majority of the issues
above

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Michael Terry]

Back to Seth I guess!

** Changed in: fwupdate (Ubuntu)
 Assignee: (unassigned) => Seth Arnold (seth-arnold)

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mario Limonciello]

It's worth mentioning that upstream saw this and has corrected many things 
upstream:
https://github.com/rhinstaller/fwupdate/issues/32#event-467050566

Also, it's available in debian-next w/ packaging here: 
http://anonscm.debian.org/cgit/uefi/fwupdate.git/log/?h=debian-next
That will be released into Debian unstable when upstream tags the next release.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Mathieu Trudel-Lapierre]

** Description changed:

  [Availability]
  fwupdate is available in universe and builds on all the architectures it is 
designed to work on (amd64, i386, armhf, arm64)
  
  [Rationale]
  fwupdate is a new EFI component for processing firmware updates for systems 
which support it. It will be used to allow users to run these upgrades, if 
their EFI BIOS allows, without the need to boot into Windows since it might not 
be available to them. This is only available for users with UEFI.
  
  [Security]
  There are no security reports open for fwupdate. However, fwupdate also ships 
an EFI binary and as such requires careful security review. Moreover, the 
fwupdate userland binary writes to ESRT tables in order to speak to the 
fwupdate EFI binary, so should benefit a security review of its own.
  
  [Quality assurance]
  Meets requirements. The package currently has bugs filed, but they are 
already handled although must wait to be fixed as they would require another 
round of signing of the EFI binary by Microsoft.
  
  [UI Standards]
  The application is translatable, although currently lacking is translations 
due to its relative newness.
  
  [Dependencies]
  All build and binary dependencies are in main.
  
  [Standards compliance]
  The package meets standards.
  
  [Maintenance]
- The package is quite small and does not require much effort on our part to 
maintain, aside from the occasional signing for the fwupdate EFI binary. It is 
maintained by the Debian EFI team in Debian; and  for Ubuntu.
+ The package is quite small and does not require much effort on our part to 
maintain, aside from the occasional signing for the fwupdate EFI binary. It is 
maintained by the Debian EFI team in Debian; and subscribed to by 
foundations-bugs for Ubuntu.
  
  [Background information]
  fwupdate is a command-line too as well as an EFI binary for use to enable and 
apply firmware updates using the UEFI Capsule Update specification, which 
allows for flashing new firmware on BIOS or system components without requiring 
the use of a specific operating system, given that it can be done directly from 
the EFI firmware interfaces.

-- 
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

[Bug 1508926] Re: [MIR] fwupdate

[Seth Arnold]

My review from mid-september. I'd also like to suggest using Coverity on this 
codebase. The 'delete_variable()' portion was handled upstream already: 
https://github.com/rhinstaller/fwupdate/commit/8139030084c51418778815ed283237553b1cec47

I also understand that Microsoft is the correct signing entity, so feel
free to ignore that too.

But fwup_set_up_update() and fwup_strerror_r() absolutely must be
addressed before we move fwupdate to main.

Thanks.



I reviewed fwupdate version 0.4-4 as checked into wily. This shouldn't be
considered a full security audit; I also did not investigate the EFI
specification or systems to validate the EFI code.

- fwupdate provides a shim to update EFI boot order and an EFI program to
  install firmware updates.
- Build-Depends: debhelper, pkg-config, libpopt-dev, libefivar-dev,
  lsb-release, gnu-efi, pesign, elfutils
- Does not daemonize
- pre/post inst/rm scripts are boilerplate
- No initscripts
- No Dbus services
- No setuid
- Provides /usr/bin/fwupdate
- No sudo fragments
- No udev rules
- No test suite; one test executable ./linux/tester.c exercises very
  little of the library/application
- No cronjobs
- Clean build logs

I'm skipping the usual summary-of-code to include only the errors and
oddities I discovered while reading the code:

- needs pesign from universe
- fwup_strerror_r() has unreachable statements, looks very buggy. Needs
  rewrite.
- fwup_set_up_update() calls free(relpath) after calling relpath =
  onstack(relpath,...). This is a severe error.
- fwup_set_up_update() calls open(O_CREAT) without specifying permissions,
  the permissions will be set to garbage
- fwup_set_up_update() fullpath memory leak via open(fullpath...) error
- fwup_set_up_update() mkostemps() call does not need O_CREAT or O_RDWR
- fwup_set_up_update() nested read()/write() loop is intricate; stdio.h
  fgetc() and fputc() loop with final fflush() might be easier to reason
  about and probably no less efficient.
- fwup_set_up_update() probably needs a rewrite -- it is doing too much
  and its memory management is too confused which leads to a severe bug.
- extensive use of stack for storage; is stack able to handle the largest
  EFI variables?
- put_info() does ssize_t arithmetic without checking for overflows -- how
  large will efidp_size() ever grow?
- get_info() free(local) may be called on uninitialized pointer if
  efi_get_variable() returns success but data_size < sizeof(*local)
- fwup_resource_iter_create() memory leak if opendir() or dirfd() fail
- utf8len() and ucs2len() handle limit=0 differently, > vs >=
- utf8len() may read beyond the end of a malformed utf8 string that is
  shorter than limit
- utf8_to_ucs2() may read beyond the end of a malformed utf8 string that
  is shorter than max or may read beyond the end of utf8+max.
- print_system_resources() and main() have user-visible strings that aren't
  localized
- delete_variable() contains unreachable code, is this a patch failure?
- All the signed files are signed by 'Red Hat Test Certificate" -- is this
  suitable for deployment?


Most of the code is written in a defensive style; however, there are
significantly troubling bugs and poorly written functions that need to
be addressed. This software also makes the assumption that stack memory
is plentiful and can handle large allocations.

Unfortunately the extensive use of stack space for storage has confused
several portions of the program; fwup_set_up_update() for example
has memory corruption issues due to calling free() on memory that was
allocated via alloca(). fwup_strerror_r() looks completely unused and
never tested.

The fwup_set_up_update() and fwup_strerror_r() functions need
work. delete_variable() needs a domain expert to determine if it is
correct. I strongly recommend that this program be run with Valgrind or
ASAN as the memory management is significantly more complicated than
usual and I may have overlooked other mistakes.

This program needs work before we promote it to main. At a minimum
fwup_set_up_update(), fwup_strerror_r() need attention. An EFI expert
needs to confirm that delete_variable() is correct. An expert needs to
confirm that the Red Hat Test Certificate is the correct certificate to
use to sign the binaries.

Thanks

-- 
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