[MIR] editorconfig-core

Bug #1984104 reported by Jeremy Bícha
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
editorconfig-core (Ubuntu)
Fix Released
Undecided
Unassigned

Bug Description

[Availability]
The package editorconfig-core is already in Ubuntu universe.
The package editorconfig-core build for the architectures it is designed to work on (all Ubuntu architectures except i386 which isn't needed)
https://launchpad.net/ubuntu/+source/editorconfig-core

[Rationale]
The package libeditorconfig0 is required in Ubuntu main for gnome-text-editor 43.

It is required in Ubuntu main no later than August 25 because that's Feature Freeze for Ubuntu 22.10. It's a desktop team goal to ship GNOME 43 as completely as possible for Ubuntu 22.10.

[Security]
- No CVEs/security issues in this software in the past

- no `suid` or `sgid` binaries
- no executables in `/sbin` and `/usr/sbin`
- 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

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

[Quality assurance - maintenance]
- The package is maintained well in Debian/Ubuntu and has no open Ubuntu bug reports except for this MIR. No serious Debian bugs.

  - Ubuntu https://bugs.launchpad.net/ubuntu/+source/editorconfig-core
  - Debian https://bugs.debian.org/cgi-bin/pkgreport.cgi?src=editorconfig-core
- The package does not deal with exotic hardware we cannot support

[Quality assurance - testing]
- The package runs a test suite on build time, if it fails
it makes the build fail, link to build log
https://launchpad.net/ubuntu/+source/editorconfig-core/0.12.5-2ubuntu2

- The package runs an autopkgtest, and is currently passing on all architectures except i386 (where it's not built)

https://autopkgtest.ubuntu.com/packages/e/editorconfig-core

https://salsa.debian.org/debian/editorconfig-core/-/blob/master/debian/tests/upstream-tests

[Quality assurance - packaging]
debian/watch is present and works

- This package does not yield massive lintian Warnings, Errors

- Please link to a recent build log of the package
https://launchpad.net/ubuntu/+source/editorconfig-core/0.12.5-2ubuntu2

- Please attach the full output you have got from `lintian --pedantic` as an extra post to this bug.

- Lintian overrides are present, but ok because it's a difference of opinion over how to handle debian/copyright formatting

https://salsa.debian.org/debian/editorconfig-core/-/blob/master/debian/source/lintian-overrides

- This package has no python2 or GTK2 dependencies

- The package will be installed by default and does not ask debconf questions

- Packaging and build is easy using dh7 style rules
https://salsa.debian.org/debian/editorconfig-core/-/blob/master/debian/rules

[UI standards]
- Application is not end-user facing (does not need translation)

[Dependencies]
- No further depends or recommends dependencies that are not yet in main
- Uses pcre2 instead of the obsolete pcre3
https://people.canonical.com/~ubuntu-archive/transitions/html/pcre2-main.html

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

[Maintenance/Owner]
- Owning Team will be Desktop Packages
- Team is already subscribed to the package

- This does not use static builds
- This does not use vendored code
- This package is not rust based
- The package has been built in the archive more recently than the last test rebuild

[Background information]
Homepage: https://editorconfig.org/

Link to upstream code:
https://github.com/editorconfig/editorconfig-core-c
and
https://github.com/editorconfig/editorconfig-core-test (shipped as a second orig tarball to fill out the tests/ directory)

CVE References

Jeremy Bícha (jbicha)
Changed in editorconfig-core (Ubuntu):
status: New → Incomplete
Jeremy Bícha (jbicha)
description: updated
Jeremy Bícha (jbicha)
description: updated
Jeremy Bícha (jbicha)
description: updated
Changed in editorconfig-core (Ubuntu):
status: Incomplete → New
Revision history for this message
Jeremy Bícha (jbicha) wrote :
description: updated
Jeremy Bícha (jbicha)
description: updated
Revision history for this message
Christian Ehrhardt  (paelzer) wrote :
Download full text (4.3 KiB)

Review for Package: editorconfig-core

[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:
- libeditorconfig0, libeditorconfig-dev
Specific binary packages built, but NOT to be promoted to main:
- editorconfig, editorconfig-doc
[They are not actively excluded, just not needed unless we seed them]

Notes:
- none
Required TODOs:
- none
Recommended TODOs:
- #1 this already has a massive build time test, it seems not too complex
  to consider adding the same (and more if more comes to mind) as autopkgtest
  to avoid a regression in release being detected late. There are not too
  many uploads of it, so issues might go undetected for quite a while otherwise.
- #2 In case it makes sense (e.g. not known unstable or incompatible with the
  target gnome version) consider moving to version 0.13 before FF.
- #3 The package should get a team bug subscriber before being promoted

[Duplication]
Various IDEs, editors and even syntax checkers will already help you to maintain
indents. But sadly none of those in main is usable in the form needed here,
as a re-usable library that will process your text.
So I'd say "There is another package in main providing the same functionality"
but none that can feasibly be used in-place for the gnome editors.
=> Ok

[Dependencies]
OK:
- no other Dependencies to MIR due to this
- no -dev/-debug/-doc packages that need exclusion (deps of libeditorconfig-dev
  are safe)
- 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 unexpected Built-Using entries
- not a go package, no extra constraints to consider in that regard
- not a rust package, no extra constraints to consider in that regard
- Does not include vendored code

Problems: None

[Security]
- 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 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)
- does not deal with cryptography (en-/decryption, certificates, signing, ...)

Problems:
- does parse data formats (xml, json, code, any text) from
  an untrusted source (people open files in their editors - 1995 style mail
  attachment exploits anyone?)
- might process arbitrary web content (open from web into editor), not fully
  arbitrary but a lack of control

=> I'm not sure how the text processing is done, due to the chance of parsing
   uncontrollable input a security review is recommended.

[Common blockers]
OK:
- does not FTBFS currently
- does have a test suite that runs at build time
  - test suite fails will fail the build upon error.
- no new python2 dependency

Problems:
- does not have test suite that ru...

Read more...

Changed in editorconfig-core (Ubuntu):
assignee: nobody → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Jeremy Bícha (jbicha) wrote :

The latest releases are included.

That's 0.12.5 for
https://github.com/editorconfig/editorconfig-core-c

And 0.13 for
https://github.com/editorconfig/editorconfig-core-test

uscan does download those versions but names them both 0.12.5. I think this is a limitation in how the dpkg multiple orig tarballs feature works.

Steve Beattie (sbeattie)
tags: added: sec-1241
Jeremy Bícha (jbicha)
description: updated
Revision history for this message
Mark Esler (eslerm) wrote :

For the most part this package looks great. Nice to have such good code comments.

Before Security Team can complete our MIR we will fuzz the conifguration file against editorconfig's use of PCRE to test for injections. Today I am off, so please expect the MIR to be posted after feature freeze.

Revision history for this message
Jeremy Bícha (jbicha) wrote :

Thanks for the follow up. Enjoy your days off!

I can file a Feature Freeze Exception to update gnome-text-editor to 43 and build against editorconfig-core if this MIR is approved later.

Revision history for this message
Jeremy Bícha (jbicha) wrote :

I should have mentioned this earlier:
gnome-text-editor just got promoted to main for Ubuntu 22.10 (previously we used a different app, gedit).
gnome-text-editor 42 has an embedded copy of editorconfig-core (using the old pcre3). gnome-text-editor 43 is using the system library (which uses the new pcre2).

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

Security recommends against including editorconfig-c anywhere in main. We are preparing a report for upstream.

Revision history for this message
Jeremy Bícha (jbicha) wrote :

Mark, gnome-text-editor 42 is already in Ubuntu main with a bundled copy of editorconfig-c. Is it ok to keep that bundled copy for Ubuntu 22.10? Next week I was going to work on trying to update gnome-text-editor to 43 with an updated bundled copy.

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

Hi Jeremy, I meant to say "for now". This is not a NACK, but I don't believe editorconfig-c will be approved for main by kinetic's release.

I will set aside time next week so that I can answer your gnome-text-editor 42 question better.

Revision history for this message
Nathan Teodosio (nteodosio) wrote (last edit ): Autopkgtest available
  • logtest Edit (210.8 KiB, text/plain; charset=UTF-8; name="logtest")

I attach an autopkgtest that runs upstream tests. It was tested
successfully in a Kinetic schroot, whose log file I also attach.

Revision history for this message
Nathan Teodosio (nteodosio) wrote : Autopkgtest: Merge request on Salsa
Revision history for this message
Jeremy Bícha (jbicha) wrote :

An autopkgtest has been added which runs the upstream build test suite so I've updated the bug description. I believe this was the only remaining Recommended todo from the original MIR review. But we're still blocked on getting the project up to the Security team's standards.

description: updated
Revision history for this message
Mark Esler (eslerm) wrote :
Download full text (4.6 KiB)

I reviewed editorconfig-core 0.12.5-2ubuntu2 as checked into kinetic and an unpackaged version containing upstream commit 41281ea to patch CVE-2023-0341. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

> EditorConfig makes it easy to maintain the correct coding style when switching between different text editors and between different projects. The EditorConfig project maintains a file format and plugins for various text editors which allow this file format to be read and used by those editors.

- CVE History:
  - CVE-2023-0341
- Bug History:
  - Some releases due to memory issues
    - https://github.com/editorconfig/editorconfig-core-c/releases
  - github issue #55 is security relevant
  - github issue #78 should be addressed
- Build-Depends?
  - lunar main
    - cmake
    - debhelper-compat (debhelper)
    - libjs-jquery (node-jquery)
      - appears to only be for editorconfig-doc
    - libpcre2-dev (pcre2)
    - pkg-config
    - linux-vdso.so.1 (kernel)
    - libpcre2-8.so.0 (pcre2)
    - libc.so.6 (glibc)
    - ld-linux-x86-64.so.2 (glibc)
  - lunar universe
    - d-shlibs
    - doxygen
    - pkg-kde-tools
- pre/post inst/rm scripts?
  - none
- init scripts?
  - none
- systemd units?
  - none
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/editorconfig-0.12.5
  - ./usr/bin/editorconfig -> editorconfig-0.12.5
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - lots of build tests
  - autopkgtests needed (!)
- cron jobs?
  - none
- Build logs:
  - trivial deprecated Doxygen warnings
  - trivial elf-error

- Processes spawned?
  - open, see File IO section
- Memory management?
  - looks good
  - see coverity
- File IO?
  - fopen in ini.c reads config file
    - Appears safe
    - Opens, reads, and closes file in single function
    - Content containing wildcard patterns read by ec_glob later
  - editorconfig.c has functions to read files and directories
  - to find a config file, editorconfig attempts to read `.editorconfig` in all directories of the path it was called from, beginning in the root directory
    - e.g., attempt to read `/.editorconfig`, then `/foo/.editorconfig`, then `/foo/bar/.editorconfig`
    - the config file furthest up the PATH containing `root = true` is the "root" config file, and previous configs should be ignored
    - https://github.com/editorconfig/editorconfig-core-c/issues/55 demonstrates how to override root config files!
    - walking the path in this manner might be analgous to CVE-2022-24765
- Logging?
  - editorconfig.c's editorconfig_get_error_msg(int err_num) contains static (safe) error messages
  - several static messages in main.c
  - in main.c, when err_num > 0, fprintf with formatting characters
  - fprintf statements do not appear susceptible to format string attacks (CAPEC-135)
- Environment variable usage?
  - none
- Use of privileged functions?
  - none
- Use of cryptography / random number sources etc?
  - none
- Use of temp files?
  - none
- Use of networking?
  - none
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- Any significant cppcheck results?
...

Read more...

Changed in editorconfig-core (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
status: New → In Progress
Revision history for this message
Jeremy Bícha (jbicha) wrote :

I updated the editorconfig-core package in Lunar to include the new CVE fix.

https://launchpad.net/ubuntu/+source/editorconfig-core/0.12.6-0.1

> It would be nice in Ubuntu renamed this package to "editorconfig-core-c"

Renaming source packages is only done rarely. I have done it before when upstream changes their project name. I don't see a strong enough need to rename the source package here now.

Jeremy Bícha (jbicha)
tags: added: lunar update-excuse
Revision history for this message
Lukas Märdian (slyon) wrote (last edit ):

All relevant action items seem to be resolved and this is already being pulled in as a dependency in -proposed. Let's set it to "Fix Committed" and subscribe ~ubuntu-archive to have it promoted.

Changed in editorconfig-core (Ubuntu):
status: In Progress → Fix Committed
Revision history for this message
Steve Langasek (vorlon) wrote :

Override component to main
editorconfig-core 0.12.6-0.1 in lunar: universe/misc -> main
editorconfig 0.12.6-0.1 in lunar amd64: universe/utils/optional/100% -> main
editorconfig 0.12.6-0.1 in lunar arm64: universe/utils/optional/100% -> main
editorconfig 0.12.6-0.1 in lunar armhf: universe/utils/optional/100% -> main
editorconfig 0.12.6-0.1 in lunar ppc64el: universe/utils/optional/100% -> main
editorconfig 0.12.6-0.1 in lunar riscv64: universe/utils/optional/100% -> main
editorconfig 0.12.6-0.1 in lunar s390x: universe/utils/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar amd64: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar arm64: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar armhf: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar i386: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar ppc64el: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar riscv64: universe/doc/optional/100% -> main
editorconfig-doc 0.12.6-0.1 in lunar s390x: universe/doc/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar amd64: universe/libdevel/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar arm64: universe/libdevel/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar armhf: universe/libdevel/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar ppc64el: universe/libdevel/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar riscv64: universe/libdevel/optional/100% -> main
libeditorconfig-dev 0.12.6-0.1 in lunar s390x: universe/libdevel/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar amd64: universe/libs/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar arm64: universe/libs/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar armhf: universe/libs/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar ppc64el: universe/libs/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar riscv64: universe/libs/optional/100% -> main
libeditorconfig0 0.12.6-0.1 in lunar s390x: universe/libs/optional/100% -> main
26 publications overridden.

Changed in editorconfig-core (Ubuntu):
status: Fix Committed → Fix Released
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.