[Xymon] Possible bug in xymon svcstatus causing segfault (buffer overrun?)

[Jeremy Laidman]

No, please ignore that. It's not the diff I was looking for. Attached is
the correct diff.

J

On Mon, 3 Jun 2024 at 09:35, Jeremy Laidman <jeremy at laidman.org> wrote:

> Mario
>
> Attached is a diff to the file that I used. It's against version 4.3.30.
>
> Cheers
> Jeremy
>
>
> On Sat, 1 Jun 2024 at 06:29, Mario <rower.master at gmail.com> wrote:
>
>>
>> Hello Jeremy/JC,
>>
>> I´m facing the same issue with switches interfaces graphs not loading in
>> trends or causing internal server error.
>> Do you have a patch to fix this?
>> Or maybe I need to downgrade xymon 4.3.30 version?
>>
>>
>> Thanks & regards,
>> Mario
>>
>>
>>
>> On Wed, Sep 13, 2023 at 4:59 AM Jeremy Laidman <jeremy at laidman.org>
>> wrote:
>>
>>> Ugh, just saw your reply in my spam folder!
>>>
>>> On Fri, 1 Sept 2023 at 23:40, J.C. Cleaver <cleaver at terabithia.org>
>>> wrote:
>>>
>>>>
>>>> On Tue, August 29, 2023 23:40, Jeremy Laidman wrote:
>>>>
>>>> >
>>>> > Elsewhere in other code, the SBUF_REALLOC macro is used to add extra
>>>> > bufferspace by calculating a new buffer size of [existing buffer size
>>>> plus
>>>> > 4k plus the length of the string being added]. So if the string to be
>>>> > added
>>>> > had 20 bytes, SBUF_REALLOC would be used to add 4k+20 bytes to the
>>>> buffer
>>>> > size. Notably, the size of the string is included in the calculation.
>>>> For
>>>> > example this bit of code elsewhere in the same file does a similar
>>>> buffer
>>>> > extension:
>>>> >
>>>> >                         if ((strlen(rrdlink) + strlen(partlink) + 1)
>>>> >=
>>>> > rrdlink_buflen) {
>>>> >                                 SBUF_REALLOC(rrdlink, rrdlink_buflen +
>>>> > strlen(partlink) + 4096);
>>>> >                         }
>>>> >                         strncat(rrdlink, partlink, (rrdlink_buflen -
>>>> > strlen(rrdlink)));
>>>> >
>>>> > Note the extra "+ strlen(<stringvar>)" bit in the extra memory being
>>>> > allocated that isn't there in the first block of code above. So my
>>>> > thinking
>>>> > is that the SBUF_REALLOC() needs to take the string's size into
>>>> account,
>>>> > like so:
>>>> >
>>>> >                                 SBUF_REALLOC(allrrdlinks,
>>>> > allrrdlinks_buflen+strlen(onelink)+4096);
>>>> >
>>>> > Is anyone with C skills able to confirm that this does indeed look
>>>> like a
>>>> > bug, and also that my proposed change is a suitable fix? After
>>>> applying
>>>> > this change and recompiling, we haven't yet seen a seg fault from
>>>> this bit
>>>> > of code, so I'm hoping it's all fixed, and without negative side
>>>> effects.
>>>> >
>>>> > Cheers
>>>> > Jeremy
>>>>
>>>> Based on my looking over it, I would confirm that this seems to be a
>>>> bug.
>>>> Specifically, it looks like we missed this part of the fix in
>>>> https://sourceforge.net/p/xymon/code/8069/?page=7#diff-1 while trying
>>>> to
>>>> get ahead of possible buffer overflows for various CVE's
>>>> (https://nvd.nist.gov/vuln/detail/CVE-2019-13486, I suppose). I can't
>>>> see
>>>> a reason not to add the length in here without a guard somewhere else
>>>> for
>>>> the length.
>>>>
>>>> Furthermore, other uses of this macro pretty much all include the length
>>>> of the incoming string or limit the concat to a specific size, which
>>>> obviously isn't ideal for URLs here.
>>>>
>>>> If you have a reproducer and your patch fixes it, I suspect that will be
>>>> sufficient and we can add it in. Can you send a coredump from a crash
>>>> off
>>>> list by any chance (using the debuginfo RPMs) just to double-check?
>>>>
>>>
>>> Sorry, can you define "reproducer"? Is that a coding term I haven't yet
>>> come across?
>>>
>>> I have core dumps, but I'll see about using the debuginfo RPMs (assuming
>>> they're on Terabithia) and get a core dump from an install of that.
>>>
>>> Thanks
>>> _______________________________________________
>>> Xymon mailing list
>>> Xymon at xymon.com
>>> http://lists.xymon.com/mailman/listinfo/xymon
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.xymon.com/pipermail/xymon/attachments/20240603/9e963788/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: File svcstatus-buffer-overrun_diff.msg
Type: application/octet-stream
Size: 75776 bytes
Desc: not available
URL: <http://lists.xymon.com/pipermail/xymon/attachments/20240603/9e963788/attachment.obj>