[bug #66052] Possible OVERRUN on two arrays in roff/troff

[Lukas]

URL:
  <https://savannah.gnu.org/bugs/?66052>

                 Summary: Possible OVERRUN on two arrays in roff/troff
                   Group: GNU roff
               Submitter: ljavorsk
               Submitted: Thu 01 Aug 2024 08:51:26 AM UTC
                Category: General
                Severity: 3 - Normal
              Item Group: None
                  Status: None
                 Privacy: Public
             Assigned to: None
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None


    _______________________________________________________

Follow-up Comments:


-------------------------------------------------------
Date: Thu 01 Aug 2024 08:51:26 AM UTC By: Lukas <ljavorsk>
There are two possible overruns of the arrays in the src/roff/troff files.
These defects were identified by SAST analyzers (combination of
coverity,snyk,cppcheck,gcc,clang,shellcheck,unicontrol), and from 98 findings
these are few that I believe are NOT false positives.

First one:
Error: OVERRUN (CWE-119):
groff-1.23.0/src/roff/troff/env.cpp:4006: alias: Assigning: "buf" = "hbuf +
1". "buf" now points to byte 1 of "hbuf" (which consists of 258 bytes).
groff-1.23.0/src/roff/troff/env.cpp:4008: cond_const: Checking "len < 256"
implies that "len" is 256 on the false branch.
groff-1.23.0/src/roff/troff/env.cpp:4033: overrun-local: Overrunning array of
258 bytes at byte offset 258 by dereferencing pointer "buf + (len + 1)".
# 4031|           // such entries are marked with a trailing space
# 4032|           buf[len] = ' ';
# 4033|->         buf[len + 1] = 0;
# 4034|           pos = (unsigned char
*)current_language->exceptions.lookup(buf);
# 4035|           if (pos != 0) {

Second one:
Error: OVERRUN (CWE-119):
groff-1.23.0/src/roff/troff/input.cpp:3651: strlen_assign: Setting variable
"n" to the return value of strlen called with argument "s".
groff-1.23.0/src/roff/troff/input.cpp:3652: alloc_strlen: Allocating
insufficient memory for the terminating null of the string.
# 3650|     else {
# 3651|       int n = strlen(s);
# 3652|->     return new temp_iterator(s, n);
# 3653|     }
# 3654|   }


Possible remedy:
Commits are in the attachments

Please let me know if you believe these are indeed false positives and why.
Thank you so much for your collaboration.






    _______________________________________________________
File Attachments:


-------------------------------------------------------
Name: 0001-Fix-the-possible-overrun-of-buf-array.patch  Size: 822B
<https://file.savannah.gnu.org/file/0001-Fix-the-possible-overrun-of-buf-array.patch?file_id=56331>
-------------------------------------------------------
Name: 0002-Fix-for-insufficient-allocation-of-iterator.patch  Size: 707B
<https://file.savannah.gnu.org/file/0002-Fix-for-insufficient-allocation-of-iterator.patch?file_id=56332>

    AGPL NOTICE

These attachments are served by Savane. You can download the corresponding
source code of Savane at
https://git.savannah.nongnu.org/cgit/administration/savane.git/snapshot/savane-89cd07edc53b91897640ed1593bee414af194b8e.tar.gz

    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?66052>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/

signature.asc
Description: PGP signature