From acd338098071bddfa1d21f87e1813727031428ea Mon Sep 17 00:00:00 2001
From: Shlomi Fish <shlomif@shlomifish.org>
Date: Fri, 1 May 2020 19:18:35 +0300
Subject: [PATCH] Detect an integer overflow when handling prcnts.

[SECURITY]

Thanks to a code review done by a facebook friend.
---
 .travis.yml                                |  2 +-
 fortune-mod/fortune/fortune.c              | 22 +++++++++++++-
 fortune-mod/tests/t/test-fortune-percent.t | 35 ++++++++++++++++++++++
 3 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 fortune-mod/tests/t/test-fortune-percent.t

diff --git a/.travis.yml b/.travis.yml
index 806f0f8..95a584a 100644
--- 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 200d190..078589b 100644
--- 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
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" );
+}