Kelson has submitted this change and it was merged.

Change subject: Fix memory corruption in decodeUrl.
......................................................................


Fix memory corruption in decodeUrl.

Reuse the function from kiwix as it does the same thing but with much
better c++ code.

If we pass a char to sscanf while letting it thinks it is a uint,
sscanf will initialize 4 bytes. As there is only byte associated to the
char, this lead to undefined behavior.

This bug was not found before as we were using the function as inline
function. Compiler optimization probably hid the error.

Change-Id: I618f8b9ede083e6580044f967153bfbe3ee3d294
---
M zimwriterfs/tools.cpp
1 file changed, 13 insertions(+), 10 deletions(-)

Approvals:
  Kelson: Verified; Looks good to me, approved



diff --git a/zimwriterfs/tools.cpp b/zimwriterfs/tools.cpp
index 49bc79e..cc6091d 100644
--- a/zimwriterfs/tools.cpp
+++ b/zimwriterfs/tools.cpp
@@ -246,19 +246,22 @@
 
 }
 
-std::string decodeUrl(const std::string &encodedUrl) {
-  std::string decodedUrl = encodedUrl;
-  std::string::size_type pos = 0;
-  char ch;
+static char charFromHex(std::string a) {
+  std::istringstream Blat(a);
+  int Z;
+  Blat >> std::hex >> Z;
+  return char (Z);
+}
 
-  while ((pos = decodedUrl.find('%', pos)) != std::string::npos &&
-        pos + 2 < decodedUrl.length()) {
-    sscanf(decodedUrl.substr(pos + 1, 2).c_str(), "%x", (unsigned int*)&ch);
-    decodedUrl.replace(pos, 3, 1, ch);
+std::string decodeUrl(const std::string &originalUrl) {
+  std::string url = originalUrl;
+  std::string::size_type pos = 0;
+  while ((pos = url.find('%', pos)) != std::string::npos &&
+        pos + 2 < url.length()) {
+    url.replace(pos, 3, 1, charFromHex(url.substr(pos + 1, 2)));
     ++pos;
   }
-
-  return decodedUrl;
+  return url;
 }
 
 std::string removeLastPathElement(const std::string& path, const bool 
removePreSeparator, const bool removePostSeparator) {

-- 
To view, visit https://gerrit.wikimedia.org/r/303540
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I618f8b9ede083e6580044f967153bfbe3ee3d294
Gerrit-PatchSet: 2
Gerrit-Project: openzim
Gerrit-Branch: master
Gerrit-Owner: Mgautierfr <mgaut...@kymeria.fr>
Gerrit-Reviewer: Kelson <kel...@kiwix.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to