Detect an integer overflow when handling prcnts.

[SECURITY]

Thanks to a code review done by a facebook friend.

diff --git a/.travis.yml b/.travis.yml
index 806f0f861879d5e8a8d8ebac43ff9449f16ddaab..95a584ab2176840aed8e8cdb198d77942efbcfbe 100644 (file)
--- a/.travis.yml
+++ b/.travis.yml
@@ -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

diff --git a/fortune-mod/fortune/fortune.c b/fortune-mod/fortune/fortune.c
index 200d190ac6efeaaa5a0caa1c2d67f99eb666f810..078589bea9ce08899e126c75f3dcaa0377e2bae8 100644 (file)
--- a/fortune-mod/fortune/fortune.c
+++ b/fortune-mod/fortune/fortune.c
@@ -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;
             }

diff --git a/fortune-mod/tests/t/test-fortune-percent.t b/fortune-mod/tests/t/test-fortune-percent.t
new file mode 100644 (file)
index 0000000..4b34f03
--- /dev/null
+++ b/fortune-mod/tests/t/test-fortune-percent.t
@@ -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" );
+}