diff mbox series

[11/13] checkpolicy: error out on parsing too big integers

Message ID 20210914124828.19488-12-cgzones@googlemail.com (mailing list archive)
State Accepted
Headers show
Series checkpolicy improvements | expand

Commit Message

Christian Göttsche Sept. 14, 2021, 12:48 p.m. UTC 
Error out instead of silently converting too big integer values in
policy sources.

    policy_parse.y:893:41: runtime error: implicit conversion from type 'unsigned long' of value 18446744073709551615 (64-bit, unsigned) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 checkpolicy/policy_parse.y | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

James Carter Sept. 14, 2021, 8:43 p.m. UTC | #1 
On Tue, Sep 14, 2021 at 8:51 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Error out instead of silently converting too big integer values in
> policy sources.
>
>     policy_parse.y:893:41: runtime error: implicit conversion from type 'unsigned long' of value 18446744073709551615 (64-bit, unsigned) to type 'unsigned int' changed the value to 4294967295 (32-bit, unsigned)
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>  checkpolicy/policy_parse.y | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
> index 6098eb50..e969d973 100644
> --- a/checkpolicy/policy_parse.y
> +++ b/checkpolicy/policy_parse.y
> @@ -890,10 +890,22 @@ filename          : FILENAME
>                         { yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) return -1; }
>                         ;
>  number                 : NUMBER
> -                       { $$ = strtoul(yytext,NULL,0); }
> +                       { unsigned long x;
> +                         errno = 0;
> +                         x = strtoul(yytext, NULL, 0);
> +                         if (errno || x > UINT_MAX)
> +                             return -1;

Some compilers will emit a warning if unsigned long is 32 bits. To
prevent this use:

if (errno)
   return -1;

#if ULONG_MAX > UINT_MAX
    if (val > UINT_MAX) {
        return -1;
    }
#endif

See commit b7ea65f547c67bfbae4ae133052583b090747e5a

And discussion:
https://lore.kernel.org/selinux/CAFftDdrGoQezmVSOnrFrPKaOnS3pejQXzYpfjwQ+QBHH_Pv02w@mail.gmail.com/

Jim



> +                         $$ = (unsigned int) x;
> +                       }
>                         ;
>  number64               : NUMBER
> -                       { $$ = strtoull(yytext,NULL,0); }
> +                       { unsigned long long x;
> +                         errno = 0;
> +                         x = strtoull(yytext, NULL, 0);
> +                         if (errno)
> +                             return -1;
> +                         $$ = (uint64_t) x;
> +                       }
>                         ;
>  ipv6_addr              : IPV6_ADDR
>                         { if (insert_id(yytext,0)) return -1; }
> --
> 2.33.0
>
diff mbox series

Patch

diff --git a/checkpolicy/policy_parse.y b/checkpolicy/policy_parse.y
index 6098eb50..e969d973 100644
--- a/checkpolicy/policy_parse.y
+++ b/checkpolicy/policy_parse.y
@@ -890,10 +890,22 @@  filename		: FILENAME
 			{ yytext[strlen(yytext) - 1] = '\0'; if (insert_id(yytext + 1,0)) return -1; }
 			;
 number			: NUMBER 
-			{ $$ = strtoul(yytext,NULL,0); }
+			{ unsigned long x;
+			  errno = 0;
+			  x = strtoul(yytext, NULL, 0);
+			  if (errno || x > UINT_MAX)
+			      return -1;
+			  $$ = (unsigned int) x;
+			}
 			;
 number64		: NUMBER
-			{ $$ = strtoull(yytext,NULL,0); }
+			{ unsigned long long x;
+			  errno = 0;
+			  x = strtoull(yytext, NULL, 0);
+			  if (errno)
+			      return -1;
+			  $$ = (uint64_t) x;
+			}
 			;
 ipv6_addr		: IPV6_ADDR
 			{ if (insert_id(yytext,0)) return -1; }