mirror of
https://github.com/qpdf/qpdf.git
synced 2024-12-22 19:08:59 +00:00
91 lines
2.7 KiB
Markdown
91 lines
2.7 KiB
Markdown
# Avoiding `operator[]`
|
|
|
|
During a security review by Red Hat security team (specifically Florian Weimer), it was discovered that qpdf used `std::string` and `std::vector`'s `operator[]`, which has no bounds checking by design. Instead, using those objects' `at()` method is preferable since it does bounds checking. Florian has a tool that can detect all uses of these methods and report them. I have a short perl script that automatically corrects any such uses. The perl script is not intended to be general, but it could be reasonably general. The only known shortcut is that it might not work very well with some cases of nested `[]`'s like `a[b[c]]` or with cases where there are line breaks inside the brackets. For qpdf's coding style, it worked on all cases reported.
|
|
|
|
To use this, obtain htcondor-analyzer, run it, and respond to the report. Here's what I did.
|
|
|
|
```
|
|
sudo aptitude install libclang-dev llvm llvm-dev clang
|
|
cd /tmp
|
|
git clone https://github.com/fweimer/htcondor-analyzer
|
|
# HEAD = 5fa06fc68a9b0677e9de162279185d58ba1e8477 at this writing
|
|
cd htcondor-analyzer
|
|
make
|
|
```
|
|
|
|
In qpdf:
|
|
|
|
```
|
|
./autogen.sh
|
|
/tmp/htcondor-analyzer/create-db
|
|
CC=/tmp/htcondor-analyzer/cc CXX=/tmp/htcondor-analyzer/cxx ./configure --disable-shared --disable-werror
|
|
# to remove conftest.c
|
|
\rm htcondor-analyzer.sqlite
|
|
/tmp/htcondor-analyzer/create-db
|
|
```
|
|
|
|
Repeat until no more errors:
|
|
|
|
```
|
|
/tmp/fix-at.pl is shown below.
|
|
```
|
|
|
|
```
|
|
make
|
|
/tmp/htcondor-analyzer/report | grep std:: | grep qpdf >| /tmp/r
|
|
perl /tmp/fix-at.pl /tmp/r
|
|
# move all *.new over the original file. patmv is my script. Can
|
|
# also use a for loop.
|
|
patmv -f s/.new// **/*.new
|
|
```
|
|
|
|
/tmp/fix-at.pl:
|
|
```perl
|
|
#!/usr/bin/env perl
|
|
require 5.008;
|
|
use warnings;
|
|
use strict;
|
|
use File::Basename;
|
|
|
|
my $whoami = basename($0);
|
|
|
|
my %to_fix = ();
|
|
while (<>)
|
|
{
|
|
chomp;
|
|
die unless m/^([^:]+):(\d+):(\d+):\s*(.*)$/;
|
|
my ($file, $line, $col, $message) = ($1, $2, $3, $4);
|
|
if ($message !~ m/operator\[\]/)
|
|
{
|
|
warn "skipping $_\n";
|
|
next;
|
|
}
|
|
push(@{$to_fix{$file}}, [$line, $col, $message]);
|
|
}
|
|
foreach my $file (sort keys %to_fix)
|
|
{
|
|
open(F, "<$file") or die;
|
|
my @lines = (<F>);
|
|
close(F);
|
|
my $last = "";
|
|
my @data = reverse sort { ($a->[0] <=> $b->[0]) || ($a->[1] <=> $b->[1]) } @{$to_fix{$file}};
|
|
foreach my $d (@data)
|
|
{
|
|
my ($line, $col) = @$d;
|
|
next if $last eq "$line:$col";
|
|
$last = "$line:$col";
|
|
die if $line-- < 1;
|
|
die if $col-- < 1;
|
|
print $lines[$line];
|
|
$lines[$line] =~ s/^(.{$col})([^\[]+)\[([^\]]+)\]/$1$2.at($3)/ or die "$file:$last\n";
|
|
print $lines[$line];
|
|
}
|
|
open(F, ">$file.new") or die;
|
|
foreach my $line (@lines)
|
|
{
|
|
print F $line;
|
|
}
|
|
close(F) or die;
|
|
}
|
|
```
|