[MIR] jigit

Bug #1978066 reported by Alexandre Ghiti
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
jigit (Ubuntu)
Won't Fix
Undecided
Lukas Märdian
usb-creator (Ubuntu)
Invalid
Undecided
Alexandre Ghiti

Bug Description

[Availability]
The package jigit is already in Ubuntu universe.
The package jigit build for the architectures it is designed to work on.
It currently builds and works for architectures: amd64, arm64, armhf, ppc64el, riscv64, s390x
Link to package [[https://launchpad.net/ubuntu/+source/jigit|jigit]]

[Rationale]
- The package jigit is required in Ubuntu main for libisofs (in turn needed for usb-creator)
- The package jigit will not generally be useful for a large part of
  our user base, but is important/helpful still because Debian is still using this file format to
  publish its releases.
- The package jigit is a new runtime dependency of package usb-creator that
  we already support

- It would be great and useful to community/processes to have the
  package jigit in Ubuntu main, but there is no definitive deadline.

[Security]
- No CVEs/security issues in this software in the past
- no `suid` or `sgid` binaries
- Binary mkjigsnap in sbin => this requires security review
- Package does not install services, timers or recurring jobs
- Packages does not open privileged ports (ports < 1024)
- Packages does not contain extensions to security-sensitive software
  (filters, scanners, plugins, UI skins, ...)

[Quality assurance - function/usage]
- The package works well right after install

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu and has not too many
  and long term critical bugs open
   - Ubuntu https://bugs.launchpad.net/ubuntu/+source/jigit/+bug
   => 2 bugs open for years, one incomplete and one briefly asking for multiarch support.
   - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=jigit
   => Only 3 that are not important but they have been open for years (one from 2004!)
- The package does not deal with exotic hardware we cannot support

[Quality assurance - testing]
- The package does not run a test at build time because no testsuite exists upstream
- The package does not run an autopkgtest because no testsuite exists upstream
- The package can be tested at autopktest time by adding a test that is actually described in libjte/test/demo.c file and that will be added before the promotion (https://launchpad.net/~alexghiti/+archive/ubuntu/riscv/+sourcepub/13673149/+listing-archive-extra)

[Quality assurance - packaging]
- debian/watch is not present and there is nothing explaining how to create the source tar: a bug report
  from 2013 proposed to add a watchfile but the maintainer refused it (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=697700)
- debian/control defines a correct Maintainer field
- This package does not yield massive lintian Warnings, Errors
- Please link to a recent build log of the package https://launchpadlibrarian.net/464118381/buildlog_ubuntu-focal-amd64.jigit_1.22-3build1_BUILDING.txt.gz
- Please attach the full output you have got from
  `lintian --pedantic` as an extra post to this bug.
$ lintian --pedantic
P: jigit source: no-dep5-copyright [debian/copyright]
P: jigit source: package-uses-old-debhelper-compat-version 10
P: jigit source: silent-on-rules-requiring-root [debian/control]
P: jigit source: trailing-whitespace debian/changelog (line 169)
P: jigit source: trailing-whitespace debian/changelog (line 226)
P: jigit source: trailing-whitespace debian/control (line 19)
P: jigit source: trailing-whitespace ... use --no-tag-display-limit to see all (or pipe to a file/program)
P: jigit source: very-long-line-length-in-source-file libjte/aclocal.m4 line 6631 is 738 characters long (>512)
P: jigit source: very-long-line-length-in-source-file libjte/configure line 10420 is 704 characters long (>512)
P: jigit source: very-long-line-length-in-source-file libjte/libtool.m4 line 6621 is 738 characters long (>512)

- Lintian overrides are not present
- This package does not rely on obsolete or about to be demoted packages.
- This package has no python2 or GTK2 dependencies
- The package will be installed by default, but does not ask debconf
  questions higher than medium
- Packaging and build is easy, link to d/rules https://sources.debian.org/src/jigit/1.22-3/debian/rules/

[UI standards]
- Application is end-user facing but no translation is present.
- End-user applications without desktop file, not needed because most users won't use it.

[Dependencies]
- No further depends or recommends dependencies that are not yet in main

[Standards compliance]
- This package correctly follows FHS and Debian Policy

[Maintenance/Owner]
- Owning Team will be Foundations
- Team is already subscribed to the package
- This does not use static builds
- This does not use vendored code
- The package successfully built during the most recent test rebuild (https://launchpad.net/ubuntu/+archive/test-rebuild-20220317-jammy/+sourcepub/13319228/+listing-archive-extra)

[Background information]
The Package description explains the package well
Upstream Name is jigit
Link to upstream project https://www.einval.com/~steve/software/JTE/
~

Related branches

description: updated
tags: added: update-excuse
Changed in usb-creator (Ubuntu):
assignee: nobody → Alexandre Ghiti (alexghiti)
Changed in jigit (Ubuntu):
assignee: nobody → Christian Ehrhardt  (paelzer)
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (4.4 KiB)

Review for Package: jigit

[Summary]
MIR team ACK under the constraint to resolve the below listed
required TODOs and as much as possible having a look at the
recommended TODOs.

This does need a security review, so I'll assign ubuntu-security

List of specific binary packages to be promoted to main: jigit
(jigit, libjte2, libjte-dev)
Specific binary packages built, but NOT to be promoted to main: libjte2, libjte-dev

Required TODOs:
- this seems rarely getting changes (which might be ok as it is just for
  legacy use) but as it wasn't built for 2 years could you please test and
  report back if it still builds fine today? Also the linked "recent
  build" was from 2 years ago. Everyone would hate to fix an issue to then
  be stopped by a rather hard unexpected FTBFS.
- Please add (as you suggested) the demo as self test at autopkgtest time
  and more if you find more candidates for testing it

Recommended TODOs:
- add d/watch file. I understand that the Debian maintainer doesn't need one
  as they unite Debian and Upstream maintainer. But as suggested in the bug
  plenty of tools and checks use it. At least you might ask again as there
  isn't a real burden having one right?
- Build warning like "Wunused-result", I wonder couldn't this be using
  Werror to generally catch more issues early on (and fix this simple case).
  Maybe give it a shot and propose it to Debian?

[Duplication]
There is no other package in main providing the same functionality.

[Dependencies]
OK:
- no other Dependencies to MIR due to this
  Although it was confusing first as there is a dependency to libio-compress-perl
  which is in universe
   libio-compress-perl | 2.105-1 | kinetic/universe | source, all
  But a more detailed check revealed that it is also provided by perl itself
  and therefore working fine. I hope that is working in the component mismatch
  check as well.
- no -dev/-debug/-doc packages that need exclusion
  There is one, but depedencies will not cause a mismatch
- No dependencies in main that are only superficially tested requiring
  more tests now.

Problems: None

[Embedded sources and static linking]
OK:
- no embedded source present
- no static linking
- does not have odd Built-Using entries
- not a go package, no extra constraints to consider in that regard
- No vendoring used, all Built-Using are in main

Problems: None

[Security]
OK:
- history of CVEs does not look concerning
- does not run a daemon as root
- does not use webkit1,2
- does not use lib*v8 directly
- does not open a port/socket
- does not process arbitrary web content
- does not use centralized online accounts
- does not integrate arbitrary javascript into the desktop
- does not deal with system authentication (eg, pam), etc)
- does not deal with security attestation (secure boot, tpm, signatures)

Problems:
- does not parse data formats (jigdo files which can be from external)
- can be used to create images which are then booted (max privilege level)

=> I think to stay on the save side, this should get a security revieww

[Common blockers]
OK:
- does not FTBFS currently
- no special HW required
- no new python2 dependency

Problems:
- does not have a test suite that runs at bu...

Read more...

Changed in jigit (Ubuntu):
assignee: Christian Ehrhardt  (paelzer) → Ubuntu Security Team (ubuntu-security)
tags: added: sec-1100
Revision history for this message
Alexandre Ghiti (alexghiti) wrote :

Required TODOs:
- @slyon sponsored the package here https://launchpad.net/ubuntu/+source/jigit/1.22-3ubuntu1 and it successfully built on all architectures and autopkgtest succeeded too: so that should close the 2 required TODOs.

Recommended TODOs:
- I opened a bug report for the watch file with a debdiff waiting for sponsoring (https://bugs.launchpad.net/ubuntu/+source/jigit/+bug/1979288) and I forwarded this to Debian again (https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=697700)
- I'll try to take a look at the warnings later (I'm on holiday next week and have a couple of things to finish before).

Thanks!

Alex

Revision history for this message
Luís Infante da Câmara (luis220413) wrote (last edit ):

When upgrading my Ubuntu 22.10 VM with apt full-upgrade, I received updates for libisoburn1 and libisofs6 (both in main) that introduce a dependency on libjte2 (in universe). I filed bug #1981359 for this.

Revision history for this message
Launchpad Janitor (janitor) wrote :

Status changed to 'Confirmed' because the bug affects multiple users.

Changed in jigit (Ubuntu):
status: New → Confirmed
Changed in usb-creator (Ubuntu):
status: New → Confirmed
Revision history for this message
Seth Arnold (seth-arnold) wrote :

Hello Luís, this is a normal part of the development process; you'll see more cases on:

https://people.canonical.com/~ubuntu-archive/component-mismatches-proposed.html
and
https://people.canonical.com/~ubuntu-archive/component-mismatches.html

Thanks

Revision history for this message
Luís Infante da Câmara (luis220413) wrote :

I filed bug #1981359 to alert users of the development release (that may not be familiar with the Ubuntu development process) for this installation issue, discouraging them from filing duplicate bugs. Therefore, I have removed the duplicate marker from that bug.

Changed in jigit (Ubuntu):
status: Confirmed → New
Changed in usb-creator (Ubuntu):
status: Confirmed → New
Changed in jigit (Ubuntu):
status: New → Incomplete
status: Incomplete → Confirmed
Changed in usb-creator (Ubuntu):
status: New → Confirmed
Revision history for this message
Sebastien Bacher (seb128) wrote :

it's also breaking the daily desktop iso build
https://launchpadlibrarian.net/612533869/buildlog_ubuntu_kinetic_amd64_ubuntu_BUILDING.txt.gz

unsure how those updates managed to migrate out of proposed with component mismatch issues...

Revision history for this message
Lukas Märdian (slyon) wrote :

usb-creator has been moved back to -proposed and I've set the "block-proposed" tag on this bug, to hold it back until jigit is ready from MIR and SEC perspectives.

tags: added: block-proposed
Revision history for this message
Mark Esler (eslerm) wrote :
Download full text (3.4 KiB)

I reviewed jigit 1.22-3ubuntu1 as checked into kinetic. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

jigit is a package of utilities to make working with jigdo files easier. jigdo files break large file, like a DVD ISO, into smaller constituent files.

- no CVE history
- build-depends on debhelper, zlib1g-dev, libbz2-dev, libio-compress-perl
- no pre/post inst/rm scripts, init scripts, systemd units, dbus services, setuid binaries, sudo fragments, polkit files, udev rules, cron jobs, WebKit, PolicyKit, nor privileged functions.
- binaries:
  - /usr/bin/jigdo-gen-checksum-list
  - /usr/bin/jigdump
  - /usr/bin/jigit-mkimage
  - /usr/bin/jigsum
  - /usr/bin/jigsum-sha256
  - /usr/bin/parallel-sums
  - /usr/sbin/mkjigsnap
- autopkgtest proposed https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1012621
- Build logs:
  - 3x "warning: ignoring return value of ‘fread’" in mkimage.c and uncompress.c
- Processes spawned:
  - in ./iso-image.pl:89, ./iso-image.pl:110, ./libjte/jte.c:189, ./libjte/jte.c:237, ./libjte/checksum.c:679
  - filename path in checksum.c main() is not checked or sanitized and can buffer overflow before system(buf)
- Memory management / File IO / Logging
  - many files manage memory, IO, and logging
  - user defined file paths are not sanitized (e.g., mkimage.c)
  - memory is not being freed
  - see ccpcheck and coverity results
- Environment variable usage
  - only in build files
- Use of cryptography:
  - no -- only checksums
- Use of temp files:
  - no -- only in build scripts and documentation/comments
- Use of networking?
  - only in, non-installed, helper script `./jigit`
- Significant cppcheck results:
  - extract-data.c:46:9: error: Resource leak: in_file [resourceLeak]
  - libjte/jte.c:649:9: error: Memory leak: checksum [memleak]
  - libjte/test/demo.c:213:9: error: Resource leak: fp_out [resourceLeak]
  - uncompress.c:157:9: error: Memory leak: comp_buf [memleak]
  - uncompress.c:165:9: error: Memory leak: comp_buf [memleak]
  - mkimage.c:1258:7: error: Used file that is not opened. [useClosedFile]
  - mkimage.c:1013:5: error: Common realloc mistake: 'buf' nulled but not freed upon failure [memleakOnRealloc]
- Significant code-c.txt:
  - unsafe input mechanisms ./mkimage.c:(602|620|630): ret = gzgets(file, buf, sizeof(buf));
- Significant Coverity results:
  - many Resource Leak (CWE-404) reports (mostly leaked storage)
  - many Uninitialized Scalar Variable (CWE-457) reports
  - many Unchecked return value from library (CWE-252) reports
  - other reports not mentioned here
- Significant shellcheck results:
  - helper shell script `./jigit` feels dubious, but does not install
    - defaults to download config from private repository
    - many ShellCheck warnings
  - libjte/bin/jigdo-gen-checksum-list:139:17: warning: For loops over find output are fragile. Use find -exec or a while read loop. [SC2044]
- Other
  - Maintainer is actively involved in FOSS and Debian \o/
  - Most code written ~17 and ~11.5 years ago
  - several "TODO" comments in libjte.c and checksum.c
  - parse_data_block() is defined in mkimage.c and jigdump.c
  - read_data_block() is defined in uncomp...

Read more...

Changed in jigit (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
Lukas Märdian (slyon) wrote :

Thanks for the security review. This leads to an interesting situation, as the reverse-depends are already in main and the component-mismatch exists.

@vorlon suggested that we should patch out the jigit parts, in order to be able to drop that dependency, as jigdo isn't used widely.

Revision history for this message
Luís Infante da Câmara (luis220413) wrote (last edit ):

Ubuntu Security Team, can you report (and possibly provide patches for) the issues mentioned in https://bugs.launchpad.net/ubuntu/+source/jigit/+bug/1978066/comments/10 to Debian?

If you say no, I will report them.

Revision history for this message
Mark Esler (eslerm) wrote :

@luis220413 can do. I will contact upstream who manages this Debian project.

Revision history for this message
Alexandre Ghiti (alexghiti) wrote :

Two packages have a dependency on jigit: libisoburn and libisofs.

I have built those 2 packages [1] without the dependency on libjte (ie jigit) and triggered libisoburn autopkgtest [2] against both packages, and this succeeded.

We could go ahead and remove those dependencies so that usb-creator, libburn, libisofs and libisoburn can be promoted to main just like @vorlon suggested, what do you think?

[1] libisofs: https://launchpad.net/~alexghiti/+archive/ubuntu/riscv/+sourcepub/13823144/+listing-archive-extra
libisoburn: https://launchpad.net/~alexghiti/+archive/ubuntu/riscv/+sourcepub/13826659/+listing-archive-extra
[2] libisofs: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-alexghiti-riscv/kinetic/amd64/libi/libisoburn/20220725_113857_8100a@/log.gz
libisoburn: https://autopkgtest.ubuntu.com/results/autopkgtest-kinetic-alexghiti-riscv/kinetic/amd64/libi/libisoburn/20220725_130557_f6e47@/log.gz

Revision history for this message
Lukas Märdian (slyon) wrote :

Yes, sounds good! Thanks for testing! Could you please provide LP/git merge-requests against the git-ubuntu repos (or debdiffs)? So I don't have to hand assemble those diffs from your PPA builds.

https://code.launchpad.net/~git-ubuntu-import/ubuntu/+source/libisofs/+git/libisofs/+ref/ubuntu/devel

https://code.launchpad.net/~git-ubuntu-import/ubuntu/+source/libisoburn/+git/libisoburn/+ref/ubuntu/devel

I'll happily sponsor this.

Changed in jigit (Ubuntu):
assignee: nobody → Lukas Märdian (slyon)
Revision history for this message
Lukas Märdian (slyon) wrote :

So the src:jigit MIR was rejected by the security team. Therefore, I'm marking this WONTFIX.

We're dropping the jigit (libjte2) dependencies from libisoburn and libisofs in order to avoid that dependency and allow the others (LP: #1977959) to migrate independently.

https://code.launchpad.net/~alexghiti/ubuntu/+source/libisoburn/+git/libisoburn/+merge/427475
https://code.launchpad.net/~alexghiti/ubuntu/+source/libisofs/+git/libisofs/+merge/427474

Changed in jigit (Ubuntu):
status: Confirmed → Won't Fix
Lukas Märdian (slyon)
Changed in usb-creator (Ubuntu):
status: Confirmed → Invalid
tags: removed: block-proposed
Lukas Märdian (slyon)
Changed in usb-creator (Ubuntu):
status: Invalid → New
tags: added: block-proposed
Lukas Märdian (slyon)
Changed in usb-creator (Ubuntu):
status: New → Invalid
tags: removed: block-proposed
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.