Bug#762609: lintian: new checks: deprecated D-Bus policies
On 2014-09-23 19:47, Simon McVittie wrote:
> Package: lintian
> Version: 2.5.27
> Severity: wishlist
> Tags: patch
>
Hi Simon,
Thanks for writing a patch for this and for including a concrete package
that triggers the tags. Could you be persuaded to write some build-time
test cases for lintian as well?
Basically, copy t/tests/basic -> t/tests/dbus-general and then:
* Edit t/tests/dbus-general/{desc,tags}
- Remember to update the "Sequence" to 6000
- Remember to add a Test-For: <tag1> <tag2>
(one tag per line please)
* Add a dbus file in t/tests/dbus-general/debian/etc/session.d/...
that triggers the tags.
* Run debian/rules runtests onlyrun=dbus
If you haven't you may also want to run:
debian/rules runtests onlyrun=suite:scripts
- It checks for various minor issues like code-style (perlcritic +
perltidy) and some Lintian specific issues. Sadly, it also
happens to run a number of irrelevant tests.
Beyond that I got some minor comments for the code as well. Find them
interleaved in the patch.
> [...]
>
> Regards,
> S
>
> [...]
>
>
> 0001-Add-checks-for-deprecated-D-Bus-policies.patch
>
>
>> [...]
> new file mode 100644
> index 0000000..1fe8475
> --- /dev/null
> +++ b/checks/dbus.desc
> @@ -0,0 +1,57 @@
> +Check-Script: dbus
> +Author: Simon McVittie <simon.mcvittie@collabora.co.uk>
> +Abbrev: dbus
Abbrev is optional and is (mostly) useless if it is not shorter than
Check-Script. [Technically it is just an alias of the check name that
people can use with e.g. -X - Accordingly it is definitely useless if
it is identical to the check name].
> +Type: binary
Given this is set to binary, the "$type" argument in "sub run" will
always be "binary". This will be important in a moment.
> +Info: Checks for deprecated or harmful D-Bus configuration
> +Needs-Info: unpacked
> +
> +[...]
No immediate remarks to the tags or their tag description.
> diff --git a/checks/dbus.pm b/checks/dbus.pm
> new file mode 100644
> index 0000000..9e9e16d
> --- /dev/null
> +++ b/checks/dbus.pm
> @@ -0,0 +1,87 @@
> +[...]
> +
> +package Lintian::dbus;
> +
> +use strict;
> +use warnings;
> +use autodie;
> +
> +use Lintian::Tags qw(tag);
> +
> +sub run {
> + my ($pkg, $type, $info) = @_;
> +
> + if ($type eq 'binary') {
>From my remark earlier, this condition is always true.
> + foreach my $file ($info->sorted_index) {
> + next if $file->is_dir;
> +
> + if ($file =~ m{^etc/dbus-1/(?:system|session).d/}) {
The dot after ")" and before "d" is not escaped.
> + my $filename = $info->unpacked($file);
> + next if -l $filename;
I appreciate the effort, but this is technically vulnerable to a DoS by
creating including a named pipe. But at least you caught the symlink
attack, which must people forget. :)
In Lintian git, we (very) recently devised a new set of methods to avoid
this kind of problem. You may want to use those instead. It looks
something to the affect of (for your case):
if ($file =~ m{...}) {
next if not $file->is_open_ok;
_check_policy($file, $file->fs_path);
}
Now the file will be opened only if it is a file (or a safe symlink to a
file). Mind you, if you want to filter out all symlinks (safe or not),
then you also need to add a "or $file->is_symlink" to the "next
if"-condition.
> [...]
> +}
> +
> +sub _check_policy {
> + my $file = shift;
> + my $filename = shift;
Arguments should use "my ($file, $filename) = @_;"
> + my $callback = shift;
This appear to be unused.
> +
> + open(my $fh, '<', $filename);
> + my $xml;
> + {
> + local $/; # read-whole-file mode
> + $xml = <$fh>;
> + }
> + close $fh;
This entire block can be replaced by:
my $xml = slurp_entire_file($filename);
(plus "use Lintian::Util qw(slurp_entire_file);" near the top).
For reference, if you needed to open the file, Lintian (in git) now
features (e.g.) $file->open for that purpose.
> [...]
> + my @rules;
> + while ($xml =~ m{(<(?:allow|deny)[^>]+send_\w+=[^>]+>)}sg) {
> + push @rules, $1;
> +[...]
Style: We are converting to: push(@rules, $1); (i.e. with parentheses).
~Niels
Reply to: