aboutsummaryrefslogtreecommitdiffstats
path: root/README-hardening.md
diff options
context:
space:
mode:
authorJay Berkenbilt <ejb@ql.org>2017-08-22 19:00:49 +0200
committerJay Berkenbilt <ejb@ql.org>2017-08-22 20:13:10 +0200
commitdba228452df014b6302eae5c5d97963fedebe6d3 (patch)
tree0aa2341b6956debe63747a164f32980c0b0047cc /README-hardening.md
parent83ec09f66c4548d356423894708e6727aaa39c88 (diff)
downloadqpdf-dba228452df014b6302eae5c5d97963fedebe6d3.tar.zst
Rename README files before converting to markdown
Diffstat (limited to 'README-hardening.md')
-rw-r--r--README-hardening.md93
1 files changed, 93 insertions, 0 deletions
diff --git a/README-hardening.md b/README-hardening.md
new file mode 100644
index 00000000..a2389af3
--- /dev/null
+++ b/README-hardening.md
@@ -0,0 +1,93 @@
+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 ----------
+#!/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;
+}
+--------------------