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

[Mario]

Hello Jeremy,

It worked! Now all the graphs are showing in trends.
Thank you very much!!


Best regards,
Mario

On Sun, Jun 2, 2024 at 9:09 PM Jeremy Laidman <jeremy at laidman.org> wrote:

> 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/ae69f8c4/attachment.htm>