Comment 13 for bug 1746629

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

I reviewed libbluray 1:1.3.2-1 as checked into kinetic. This shouldn't be considered a full audit but rather a quick gauge of maintainability.

> Libbluray is an open-source library designed for Blu-Ray Discs playback for media players, like VLC or MPlayer.

Upstream: https://code.videolan.org/videolan/libbluray/

- CVE History:
  - trivial
  - CVE-2015-7810
    - "libbluray MountManager class has a time-of-check time-of-use (TOCTOU) race when expanding JAR files"
    - upstream never vulnerable with default build settings
    - ./debian/rules:9:confflags_java = --enable-bdjava-jar
    - see https://www.openwall.com/lists/oss-security/2015/10/12/7
- Build-Depends?
  - ld-linux-x86-64.so.2
  - libbluray.so.2
  - libbrotlicommon.so.1
  - libbrotlidec.so.1
  - libc.so.6
  - libexpat.so.1
  - libfontconfig.so.1
  - libfreetype.so.6
  - libgcc_s.so.1
  - libicudata.so.70
  - libicuuc.so.70
  - liblzma.so.5
  - libm.so.6
  - libpng16.so.16
  - libstdc++.so.6
  - libudfread.so.0
  - libuuid.so.1
  - libxml2.so.2
  - libz.so.1
  - linux-vdso.so.1
- pre/post inst/rm scripts?
  - none
- init scripts?
  - none
- systemd units?
  - none
- dbus services?
  - none
- setuid binaries?
  - none
- binaries in PATH?
  - ./usr/bin/bd_info
  - ./usr/bin/bd_list_titles
  - ./usr/bin/bd_splice
- sudo fragments?
  - none
- polkit files?
  - none
- udev rules?
  - none
- unit tests / autopkgtests?
  - MISSING !!!
- cron jobs?
  - none
- Build logs:
  - clean logs--lgtm

- Processes spawned?
  - mutex.c defines <pthread.h> initialization, locking and destruction
  - use of Xlets
- Memory management?
  - heavy memory use
  - ~300 uses of memcpu, strcpy, sprintf, calloc, etc outside of build/documentation
  - what I examined looked okay
  - see cppcheck results below
- File IO?
  - heavy IO use
  - functions defined mostly in src/file/* and bluray.c
  - paths are being checked
- Logging?
  - heavy logging use
  - many debug/error messages (+1500)
    - log overflows possible
  - xine, java, and rest of codebase use different logging methods
  - see logging.c
- Environment variable usage?
  - sanitized
    - see dirs_xdg.c and strutl.h
- Use of privileged functions?
  - yes, in Java
  - import java.security.PrivilegedAction and other java.security.*
  - primarily defined in BDJSecurityManager.java
- Use of cryptography / random number sources etc?
  - no, only video codecs
- Use of temp files?
  - CacheDir.java defines CacheDir object used by other Java files.
- Use of networking?
  - socket use appears to be locked to "bd://"
- Use of WebKit?
  - none
- Use of PolicyKit?
  - none

- any significant cppcheck results? (checked C)
  - memleakOnRealloc in bd_info
    - https://code.videolan.org/videolan/libbluray/-/merge_requests/32
    - nb: src/examples/bd_info.c -> /usr/bin/bd_info
- any significant Coverity results? (checked C and Java)
  - null dereferences in asm's MethodWriter.java and ClassReader.java
    - e.g., ClassReader.java:1795, 'name' parameter is null and if av != null, av.vist() will reference null
  - usage of large stack frame often
    - +32k frame allocations
      - e.g., disc.c:disc_cache_bdrom_file
    - none of installed /usr/bin/ have -fstack-clash-protection instructions
    - /usr/bin/bd_list_titles is not stack protected
- any signficant flawfinder results? (checked C)
  - 338 total hits
  - what I checked were false positives

- Evangelion's "org.videolan.backdoor"
  - src/libbluray/bdj/java/org/videolan/backdoor/System.java sounds dubious
  - ReplaceMethodPatcher is an extraodinarily powerful class
    - "Implement overriding method calls in Xlets"
    - use of this class must be monitored tightly future misuse
  - relevant commits are 1ce479c1cfa1dfdb33a2f150e91edaf28af364e4 and 824d87f452d306715a85b9d5e81415c209797186 from 2020-09-18
  - After examination and conferring with other Security Members this code appears to be safe and works as commented

This is a complex codebase which uses low level C and Java. It contains third party projects and libraries. There is heavy use of memory, IO, & logging.

Security team cautiously ACK for promoting libbluray to main on the strict conditions that (1) autopkgtests and build tests are added as suggested by @paslzer (2) add test to check if instances of ReplaceMethodPatcher are added or modified in codebase.