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

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

Closed
shamefulCake1 opened this issue Jun 21, 2024 · 3 comments · Fixed by #1752
Closed

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

shamefulCake1 opened this issue Jun 21, 2024 · 3 comments · Fixed by #1752
Labels
Milestone

Comments

@shamefulCake1
Copy link

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
Copy link
Collaborator

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

@klayoutmatthias klayoutmatthias added this to the 0.29.3 milestone Jun 22, 2024
klayoutmatthias pushed a commit that referenced this issue Jun 22, 2024
@klayoutmatthias klayoutmatthias linked a pull request Jun 22, 2024 that will close this issue
@sebastian-goeldi
Copy link
Contributor

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
Copy link
Author

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.

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

Successfully merging a pull request may close this issue.

3 participants