REST POST buffer overflow

adrian.p.smith at bt.com adrian.p.smith at bt.com
Tue Jan 12 16:01:56 CET 2021


Thanks for the quick response Arran.

I think we will write an integration test to reproduce the issue and then decide which approach works for us.

Adrian


-----Original Message-----
From: Freeradius-Users <freeradius-users-bounces+adrian.p.smith=bt.com at lists.freeradius.org> On Behalf Of Arran Cudbard-Bell
Sent: 12 January 2021 14:36
To: FreeRadius users mailing list <freeradius-users at lists.freeradius.org>
Subject: Re: REST POST buffer overflow



> On Jan 12, 2021, at 1:48 PM, adrian.p.smith at bt.com wrote:
> 
> We are running 3.0.15 and are seeing occasional issues when a large accounting packet is passed to our REST server:
> 
> It appears that a buffers gets a bit full and if this occurs when an attribute name is written, but not the value, the next chunk starts with the value of the NEXT AVP instead of the one that was being processed when the buffer got full:

That code is ~9 years old and I believe you're the first person to hit that bug, congratulations!

It happens when the value has been written, there's no bytes remaining in the output buffer, and there are more attributes to encode.  Which may not sound that uncommon, but the length of the value needs to fit perfectly in the output buffer in order for this to happen.

The correct fix is to add an encoding state to represent the separator '&', but I don't want to accidentally break anything else.

The bad but functional fix is to check if we can write the separator before advancing the pair cursor.

https://eur02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FFreeRADIUS%2Ffreeradius-server%2Fcommit%2F3dd36aa1058343e3e7a99ca29ffaae4c172748e1&data=04%7C01%7Cadrian.p.smith%40bt.com%7C846324fd74e54102233d08d8b70774ed%7Ca7f356889c004d5eba4129f146377ab0%7C0%7C0%7C637460590378734877%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=2rOuggVWPCziI7pymMwMuK5ioGKtNadU09CpedXr9i4%3D&reserved=0

With the fix applied you'll notice that the debug output indicates the value has been encoded twice, but in reality it'll only occur in the POST data once.

Because that code has been stable for so long you might just be able to apply the diff directly to 3.0.15, or you can wait for the next stable release and upgrade.

There's also a bit of a crappy work around.  If you adjust the 'chunk' parameter for a given section (maybe set it to 900) you can reduce the amount of buffer that's filled before the encoding callback returns.  As this is such an edgy edge case it's likely that you won't hit it with a different output buffer size.

-Arran


Arran Cudbard-Bell <a.cudbardb at freeradius.org> FreeRADIUS Development Team

FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2




More information about the Freeradius-Users mailing list