octoleo/qpdf
2
1
Fork 0
mirror of https://github.com/qpdf/qpdf.git synced 2024-12-22 02:49:00 +00:00
Code Issues Releases Wiki Activity

Fix string bounds checking in completion code (fixes #441)

Browse Source
This commit is contained in:
Jay Berkenbilt 2021-05-12 08:55:10 -04:00
parent 3cacb27a90
commit df38fe8e48
7 changed files with 43 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
ChangeLog
View File

@ -1,3 +1,9 @@
2021-05-12 Jay Berkenbilt <ejb@ql.org>
* Bug fix: ensure we don't overflow any string bounds while
handling completion, even when we are given bogus input values.
Fixes #441.
2021-05-09 Jay Berkenbilt <ejb@ql.org>
* Improve performance of preservation of object streams by

28
qpdf/qpdf.cc
View File

@ -3469,16 +3469,27 @@ ArgParser::checkCompletion()
{
// See if we're being invoked from bash completion.
std::string bash_point_env;
if (QUtil::get_env("COMP_LINE", &bash_line) &&
QUtil::get_env("COMP_POINT", &bash_point_env))
// On Windows with mingw, there have been times when there appears
// to be no way to distinguish between an empty environment
// variable and an unset variable. There are also conditions under
// which bash doesn't set COMP_LINE. Therefore, enter this logic
// if either COMP_LINE or COMP_POINT are set. They will both be
// set together under ordinary circumstances.
bool got_line = QUtil::get_env("COMP_LINE", &bash_line);
bool got_point = QUtil::get_env("COMP_POINT", &bash_point_env);
if (got_line || got_point)
{
size_t p = QUtil::string_to_uint(bash_point_env.c_str());
if ((p > 0) && (p <= bash_line.length()))
if (p < bash_line.length())
{
// Truncate the line. We ignore everything at or after the
// cursor for completion purposes.
bash_line = bash_line.substr(0, p);
}
if (p > bash_line.length())
{
p = bash_line.length();
}
// Set bash_cur and bash_prev based on bash_line rather than
// relying on argv. This enables us to use bashcompinit to get
// completion in zsh too since bashcompinit sets COMP_LINE and
@ -3489,8 +3500,9 @@ ArgParser::checkCompletion()
// for the first separator. bash_cur is everything after the
// last separator, possibly empty.
char sep(0);
while (--p > 0)
while (p > 0)
{
--p;
char ch = bash_line.at(p);
if ((ch == ' ') || (ch == '=') || (ch == ':'))
{
@ -3498,7 +3510,10 @@ ArgParser::checkCompletion()
break;
}
}
bash_cur = bash_line.substr(1+p, std::string::npos);
if (1+p <= bash_line.length())
{
bash_cur = bash_line.substr(1+p, std::string::npos);
}
if ((sep == ':') || (sep == '='))
{
// Bash sets prev to the non-space separator if any.
@ -3512,8 +3527,9 @@ ArgParser::checkCompletion()
// Go back to the last separator and set prev based on
// that.
size_t p1 = p;
while (--p1 > 0)
while (p1 > 0)
{
--p1;
char ch = bash_line.at(p1);
if ((ch == ' ') || (ch == ':') || (ch == '='))
{

13
qpdf/qtest/qpdf.test
View File

@ -86,6 +86,10 @@ $td->runtest("UTF-16 encoding errors",
$td->NORMALIZE_NEWLINES);
my @completion_tests = (
['', 0, 'bad-input-1'],
['', 1, 'bad-input-2'],
['', 2, 'bad-input-3'],
['qpdf', 2, 'bad-input-4'],
['qpdf ', undef, 'top'],
['qpdf -', undef, 'top-arg'],
['qpdf --enc', undef, 'enc'],
@ -5229,8 +5233,13 @@ sub bash_completion
$point = length($line);
}
my $before_point = substr($line, 0, $point);
$before_point =~ m/^(.*)([ =])([^= ]*)$/ or die;
my ($first, $sep, $cur) = ($1, $2, $3);
my $first = '';
my $sep = '';
my $cur = '';
if ($before_point =~ m/^(.*)([ =])([^= ]*)$/)
{
($first, $sep, $cur) = ($1, $2, $3);
}
my $prev = ($sep eq '=' ? $sep : $first);
$prev =~ s/.* (\S+)$/$1/;
my $this = $first;

1
qpdf/qtest/qpdf/completion-bad-input-1.out Normal file
View File

@ -0,0 +1 @@
!

1
qpdf/qtest/qpdf/completion-bad-input-2.out Normal file
View File

@ -0,0 +1 @@
!

1
qpdf/qtest/qpdf/completion-bad-input-3.out Normal file
View File

@ -0,0 +1 @@
!

1
qpdf/qtest/qpdf/completion-bad-input-4.out Normal file
View File

@ -0,0 +1 @@
!