Re: Patch: fix various issues found by static analysis [LONG_MAX]

[Vitezslav Crhonek]

On 10/15/24 1:06 PM, Gavin Smith wrote:

On Tue, Oct 15, 2024 at 12:36:59PM +0200, Vitezslav Crhonek wrote:

Hi,

Please review proposed fixes of issues found by static analysis
of texinfo-7.1 in the attached patch and consider applying them.

Thanks for sending them, I would like to take some time to look
at and understand them.

Sure, no problem.

 From e807d95e3422b1b45b6ec9d3b6b0f559c136fa5f Mon Sep 17 00:00:00 2001
From: Vitezslav Crhonek <vcrho...@redhat.com>
Date: Tue, 15 Oct 2024 10:51:07 +0200
Subject: [PATCH 1/7] * info/makedoc.c: fix possible integer overflow

---
  info/makedoc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/info/makedoc.c b/info/makedoc.c
index 6810d4d228..9f525df110 100644
--- a/info/makedoc.c
+++ b/info/makedoc.c
@@ -296,7 +296,7 @@ process_one_file (char *filename, FILE *doc_stream, FILE 
*funs_stream)
        char *func, *doc;
        char *func_name;

- for (; offset < (file_size - decl_len); offset++)

+      for (; offset < (file_size - decl_len) && offset < (LONG_MAX - 
decl_len); offset++)
          {
            if (buffer[offset] == '\n')
              {

Do we really need to check for integer overflow?  Surely every C program
in existence has possible integer overflows if its input gets big enough
and I don't want comparisons to LONG_MAX etc. everywhere.

The check

    offset < (file_size - decl_len) && offset < (LONG_MAX - decl_len)

is strange.  One of the two values that offset is compared to is as least
as large as the other (and files_size and decl_len don't change values
throughout the loop).

Suppose

    (file_size - decl_len) > (LONG_MAX - decl_len)

Then

    file_size > LONG_MAX

This is impossible as file_size is a long.

So (file_size - decl_len) <= (LONG_MAX - decl_len) and the part you
added is unnecessary.

I would agree that this shouldn't be real problem and if you want to
omit this hunk, I'm fine with that.

Vita