[MIR] editorconfig-core

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

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.

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.

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.

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

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

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.

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.

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

https://salsa.debian.org/debian/editorconfig-core/-/merge_requests/8

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.

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

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.

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.

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.