Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Found memory leak in Radius.decode_attributes method (1.0.3) #31

Closed
markhorsman opened this issue Jul 21, 2016 · 1 comment
Closed

Found memory leak in Radius.decode_attributes method (1.0.3) #31

markhorsman opened this issue Jul 21, 2016 · 1 comment

Comments

@markhorsman
Copy link
Contributor

Dear colleague developer,

We (my team and I) are using your radius library for a while now and I must say we are very happy with it.
It work very fast and gets the job done. We use it to process thousands of requests per minute and it does it like a boss.

A couple of weeks ago we started working on implementing webRTC in one of our web applications.
To get it working we send a JWT (JSON Webtoken) token to our SIP server (Brekeke) along with the register request using the JsSIP javascript library.
We do this because we don't want to show the password of the SIP account in plain text in our client-side application. We use the Vendor-Specific attributes (dictionaries) to pass this JWT token to our node radius process. We then validate the JWT token in our authentication process after we decoded the RADIUS packet using your library. This worked fine and we could register the SIP account and make calls and everything.

But here comes the problem. We noticed that sometimes our SIP server sends malformed RADIUS packets to our radius service (random). And we know we have to fix this on our side but something happened in the decode process of your radius lib. It created a memory leak causing the node app to crash.

This memory leak occurs when the length variable in the decode_attributes function is 0.
Because the attributes are decoded inside a while loop:
while (data.length > 0) / data = data.slice(length);
and the length variable used to slice the data is 0, it will stay in this loop untill the raw_attrs variable is so big, it eats up too much memory.

Off course we can easily fix this with the following line:

if (!length) throw new Error("readUInt8(1) has length of 0");

You can reproduce this issue using a buffer I provided:
corrupBuffer.txt

I hope we can discuss this problem and hopefully come to a solution.

With regards,

Mark Horsman
Tam One

@markhorsman
Copy link
Contributor Author

I created a pull request for this issue:

#32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant