Lintian – Upcoming API making it easier to write correct and safe code

The upcoming version of Lintian will feature a new set of API that attempts to promote safer code. It is hardly a “ground-breaking discovery”, just a much needed feature.

The primary reason for this API is that writing safe and correct code is simply too complicated that people get it wrong (including yours truly on occasion).   The second reason is that I feel it is a waste having to repeat myself when reviewing patches for Lintian.

Fortunately, the kind of issues this kind of mistake creates are usually minor information leaks, often with no chance of exploiting it remotely without the owner reviewing the output first[0].

Part of the complexity of writing correct code originates from the fact that Lintian must assume Debian packages to be hostile until otherwise proven[1]. Consider a simplified case where we want to read a file (e.g. the copyright file):

package Lintian::cpy_check;
use strict; use warnings; use autodie;
sub run {
  my ($pkg, undef, $info) = @_;
  my $filename = "usr/share/doc/$pkg/copyright";
  # BAD: This is an example of doing it wrong
  open(my $fd, '<', $info->unpacked($filename));
  ...;
  close($fd);
  return;
}

This has two trivial vulnerabilities[2].

1. Any part of the path (usr,usr/share, …) can be asymlink to “somewhere else” like /
   1. Problem: Access to potentially any file on the system with the credentials of the user running Lintian.  But even then, Lintian generally never write to those files and the user has to (usually manually) disclose the report before any information leak can be completed.
2. The target path can point to a non-file.
   1. Problem: Minor inconvenience by DoS of Lintian.  Examples include a named pipe, where Lintian will get stuck until a signal kills it.

Of course, we can do this right[3]:

package Lintian::cpy_check;
use strict; use warnings; use autodie;
use Lintian::Util qw(is_ancestor_of);
sub run {
  my ($pkg, undef, $info) = @_;
  my $filename = "usr/share/doc/$pkg/copyright";
  my $root = $info->unpacked
  my $path = $info->unpacked($filename);
  if ( -f $path and is_ancestor_of($root, $path)) {
    open(my $fd, '<', $path);
    ...;
    close($fd);
  }
  return;
}

Where “is_ancestor_of” is the only available utility to assist you currently.  It hides away some 10-12 lines of code to resolve the two paths and correctly asserting that $path is (an ancestor of) $root.  Prior to Lintian 2.5.12, you would have to do that ancestor check by hand in each and every check[4].

In the new version, the correct code would look something like this:

package Lintian::cpy_check;
use strict; use warnings; use autodie;
sub run {
  my ($pkg, undef, $info) = @_;
  my $filename = "usr/share/doc/$pkg/copyright";
  my $path = $info->index_resolved_path($filename);
  if ($path and $path->is_open_ok) {
    my $fd = $path->open;
    ...;
    close($fd);
  }
  return;
}

Now, you may wonder how that promotes safer code.  At first glance, the checking code is not a lot simpler than the previous “correct” example.  However, the new code has the advantage of being safer even if you forget the checks.  The reasons are:

1. The return value is entirely based on the “file index” of the package (think: tar vtf data.tar.gz).  At no point does it use the file system to resolve the path.  Whether your malicious package trigger an undef warning based on the return value of index_resolved_path leaks nothing about the host machine.
   1. However, it does take safe symlinks into account and resolves them for you.  If you ask for ‘foo/bar’ and ‘foo’ is a symlink to ‘baz’ and ‘baz/bar’ exists in the package, you will get ‘baz/bar’.  If ‘baz/bar’ happens to be a symlink, then it is resolved as well.
   2. Bonus: You are much more likely to trigger the undef warning during regular testing, since it also happens if the file is simply missing.
2. If you attempt to call “$path->open” without calling “$path->is_open_ok” first, Lintian can now validate the call for you and stop it on unsafe actions.

It also has the advantage of centralising the code for asserting safe access, so bugs in it only needs to be fixed in one place.  Of course, it is still possible to write unsafe code.  But at least, the new API is safer by default and (hopefully) more convenient to use.

—

[0] Lintian.debian.org being the primary exception here.

[1] This is in contrast to e.g. piuparts, which very much trusts its input packages by handing the package root access (albeit chroot’ed, but still).

[2] And also a bug.  Not all binary packages have a copyright – instead some while have a symlink to another package.

[3] The code is hand-typed into the blog without prior testing (not even compile testing it).  The code may be subject to typos, brown-paper-bag bugs etc. which are all disclaimed (of course).

[4] Fun fact, our documented example for doing it “correctly” prior to implementing is_ancestor_of was in fact not correct.  It used the root path in a regex (without quoting the root path) – fortunately, it just broke lintian when your TMPDIR / LINTIAN_LAB contained certain regex meta-characters (which is pretty rare).