KLayout / klayout

KLayout Main Sources
http://www.klayout.org
GNU General Public License v3.0
743 stars 192 forks source link

[patch] `tlXMLParser.cc:cdata_handler` buffer overflow. #1751

Closed shamefulCake1 closed 6 days ago

shamefulCake1 commented 2 weeks ago

Hello,

in cdata_handler there is a call like this:

  d->cdata (std::string (s, 0, size_t (len)));

this will not work, because the prototype (char* , int start, unsigned int number) does not exist. It will be implicitly cast to (std::string, int start, unsigned int number) , so it will run std::string constructor, which is a disaster, because s is not 0-terminated, so the strlen embedded into std::string constructor will run until the end of memory.

The following patch fixes the problem:

diff --git a/src/tl/tl/tlXMLParser.cc b/src/tl/tl/tlXMLParser.cc
index 164578ab0..b74280b4b 100644
--- a/src/tl/tl/tlXMLParser.cc
+++ b/src/tl/tl/tlXMLParser.cc
@@ -343,7 +343,7 @@ void end_element_handler (void *user_data, const XML_Char *name)
 void cdata_handler (void *user_data, const XML_Char *s, int len)
 {
   XMLParserPrivateData *d = reinterpret_cast<XMLParserPrivateData *> (user_data);
-  d->cdata (std::string (s, 0, size_t (len)));
+  d->cdata (std::string (s, size_t (len)));
 }

apply it with patch -p0 < patchname.patch.

I cannot make pull requests on github, because that requires activating 2FA, and I didn't manage to make it work.

klayoutmatthias commented 1 week ago

Thanks. This is also EXPAT-only.

But enabling 2FA isn't a big problem. It will give you more credibility. But the patch definitely makes sense.

Matthias

sebastian-goeldi commented 1 week ago

Nothing to do with the issue itself. Just wanted to let you know for 2FA. You can use other programs than android/ios apps. If you checkout https://docs.github.com/en/authentication/securing-your-account-with-two-factor-authentication-2fa/configuring-two-factor-authentication#configuring-two-factor-authentication-using-a-totp-app , there is a part about "setup key" link; this will allow you to get the TOTP secret without having to go through the QR code. Using this key will allow you to use other TOTP managers such as the desktop app of BitWarden and similar. My personal favorite is KeePassXC (with browser plugin). With this you aren't dependent on any google/apple platform (assuming that is your problem). If that's not your primary concern but failed somewhere else, let me know, I'd be happy to jump on a quick call or similar to help you set it up.

Tl;dr 2FA really shouldn't be the thing stopping you from contributing and it would be a shame if that's the road block.

shamefulCake1 commented 1 week ago

No. I already "have" an account that is permanently unreachable because of the fubar'ed 2FA, and recently I found that some of the repos still associated with it have "personal data". I intend to NEVER repeat this experience.