Skip to content
This repository has been archived by the owner on Jan 13, 2022. It is now read-only.

So many buffer overflows. Can't count. #2

Open
altf4 opened this issue Jul 7, 2014 · 6 comments
Open

So many buffer overflows. Can't count. #2

altf4 opened this issue Jul 7, 2014 · 6 comments

Comments

@altf4
Copy link

altf4 commented Jul 7, 2014

Did a quick:

grep -rn "sprintf(" .

and got 215 results. Some of these seem pretty serious, too. Like:

https://github.com/OculusVR/RakNet/blob/master/Source/EmailSender.cpp#L153

Which has a good chance of being remote.

I may try to make a pull request later, but this is going to need a big change and I thought you ought to know soon. "Swiss cheese" comes to mind. I'd recommend taking care of the low hanging fruit first, replace all unsafe string functions with their safe counterparts. IE: snprintf() instead of sprintf(). I'd also not recommend running this on any important systems until then.

Other things to grep for which will give you loads of dangerous overflows in the RakNet code:

"strcat", "strcpy", and "memcpy"

Thanks

@phoem
Copy link

phoem commented Jul 7, 2014

Ya I've been finding these too, started patching for them, however have no way to test. Anyone interested in working with me/testing before I submit patches?

@phoem
Copy link

phoem commented Jul 7, 2014

Also not really a fan of strlen() all sizes should be tracked instead, should never need to use strlen()

@phoem
Copy link

phoem commented Jul 7, 2014

As a rule of thumb i take the unsafe functions and redefine them to garbage so that if I do use one by mistake, compiling yells at me

@danoli3
Copy link

danoli3 commented Jul 9, 2014

This is most likely due to the pre-C++11 IDE / compiler support.
If you want to make a PR, wrap the new functions in C++11 macros and keep the "unsafe"

#if __cplusplus > 199711L
    // C++11 +
#else
  // pre-C++11 or not defined by compiler.
#endif

@milesrout
Copy link

Is this going to be fixed? Has it been fixed? A project I am working on wants to use this as a dependency but we cannot until we know it doesn't have (known) vulnerabilities.

@atuljangra
Copy link

As far as I know, no one is working on this currently.

rhard pushed a commit to rhard/RakNet that referenced this issue Oct 26, 2017
Resolve Issue facebookarchive#8 - Incorrect/duplicated test conditional
g-andrade added a commit to g-andrade/RakNet that referenced this issue Jun 19, 2019
…support_android_arm64

Remove unnecessary include for android builds
g-andrade pushed a commit to g-andrade/RakNet that referenced this issue Oct 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants