Skip to content

Commit

Permalink
Detect an integer overflow when handling prcnts.
Browse files Browse the repository at this point in the history
[SECURITY]

Thanks to a code review done by a facebook friend.
  • Loading branch information
shlomif committed May 1, 2020
1 parent 736705a commit acd3380
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 2 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ addons:
before_install:
- cpanm local::lib
- eval "$(perl -Mlocal::lib=$HOME/perl_modules)"
- cpanm Code::TidyAll::Plugin::ClangFormat Code::TidyAll::Plugin::Flake8 Code::TidyAll::Plugin::TestCount File::Find::Object List::Util Path::Tiny Perl::Critic Perl::Tidy Test::Code::TidyAll Test::Differences Test::RunValgrind
- cpanm Code::TidyAll::Plugin::ClangFormat Code::TidyAll::Plugin::Flake8 Code::TidyAll::Plugin::TestCount File::Find::Object List::Util Path::Tiny Perl::Critic Perl::Tidy Test::Code::TidyAll Test::Differences Test::RunValgrind Test::Trap
cache:
directories:
- $HOME/perl_modules
Expand Down
22 changes: 21 additions & 1 deletion fortune-mod/fortune/fortune.c
Original file line number Diff line number Diff line change
Expand Up @@ -917,12 +917,32 @@ static int form_file_list(char **files, int file_cnt)
sp = files[i];
else
{
const int MAX_PERCENT = 100;
bool percent_has_overflowed = false;
percent = 0;
for (sp = files[i]; isdigit(*sp); sp++)
{
percent = percent * 10 + *sp - '0';
if (percent > 100)
percent_has_overflowed = (percent > MAX_PERCENT);
if (percent_has_overflowed)
{
break;
}
}
if (percent_has_overflowed || (percent > 100))
{
fprintf(stderr, "percentages must be <= 100\n");
fprintf(stderr,
"Overflow percentage detected at argument \"%s\"!\n",
files[i]);
ErrorMessage = true;
return false;
}
if (percent < 0)
{
fprintf(stderr,
"Overflow percentage detected at argument \"%s\"!\n",
files[i]);
ErrorMessage = true;
return false;
}
Expand Down
35 changes: 35 additions & 0 deletions fortune-mod/tests/t/test-fortune-percent.t
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/usr/bin/perl

use strict;
use warnings;
use 5.014;

use FindBin;
use lib "$FindBin::Bin/lib";
use FortTestInst ();
use Test::More tests => 2;
use Test::Trap
qw( trap $trap :flow:stderr(systemsafe):stdout(systemsafe):warn );

{
my $inst_dir = FortTestInst::install("fortune-percent-overflow");
my $IS_WIN = ( $^O eq "MSWin32" );
my @cmd = (
$inst_dir->child( 'games', 'fortune' ),
"999999999999999%", "songs-poems"
);

print "Running [@cmd]\n";
trap
{
system(@cmd);
};

# TEST
like( $trap->stderr(),
qr/Overflow percentage detected at argument "999999999999999%"!/,
"right error." );

# TEST
unlike( $trap->stderr(), qr/-[0-9]/, "negative integer" );
}

0 comments on commit acd3380

Please sign in to comment.