Bug fix (kb.chars generation collision)

doc/CHANGELOG.md

@@ -84,6 +84,7 @@
 * Removed the time outliers from the time statistics.
 * Improved the detection of the SQL dialect of the target.
 * The switch `--xpath` no longer reports an injection when the page changes only because it shows the payload again. Reflection is not proof that an expression was evaluated.
+* sqlmap now makes sure that the random markers around the retrieved data are all different. Two equal markers made the error-based technique give back the wrong character.
 
 ## Quality
 

lib/core/option.py

@@ -2193,9 +2193,16 @@ def _setKnowledgeBaseAttributes(flushAll=True):
 
     kb.chars = AttribDict()
     kb.chars.delimiter = randomStr(length=6, lowercase=True)
-    kb.chars.start = "%s%s%s" % (KB_CHARS_BOUNDARY_CHAR, randomStr(length=3, alphabet=KB_CHARS_LOW_FREQUENCY_ALPHABET), KB_CHARS_BOUNDARY_CHAR)
-    kb.chars.stop = "%s%s%s" % (KB_CHARS_BOUNDARY_CHAR, randomStr(length=3, alphabet=KB_CHARS_LOW_FREQUENCY_ALPHABET), KB_CHARS_BOUNDARY_CHAR)
-    kb.chars.at, kb.chars.space, kb.chars.dollar, kb.chars.hash_ = ("%s%s%s" % (KB_CHARS_BOUNDARY_CHAR, _, KB_CHARS_BOUNDARY_CHAR) for _ in randomStr(length=4, lowercase=True))
+    # NOTE: markers have to be mutually distinct (e.g. equal start/stop makes the delimited output ambiguous, while equal replacement markers make _errorReplaceChars() restore the wrong character)
+    _ = set()
+    while len(_) < 2:
+        _.add(randomStr(length=3, alphabet=KB_CHARS_LOW_FREQUENCY_ALPHABET))
+    kb.chars.start, kb.chars.stop = ("%s%s%s" % (KB_CHARS_BOUNDARY_CHAR, __, KB_CHARS_BOUNDARY_CHAR) for __ in _)
+
+    _ = set()
+    while len(_) < 4:
+        _.add(randomStr(length=1, lowercase=True))
+    kb.chars.at, kb.chars.space, kb.chars.dollar, kb.chars.hash_ = ("%s%s%s" % (KB_CHARS_BOUNDARY_CHAR, __, KB_CHARS_BOUNDARY_CHAR) for __ in _)
 
     kb.checkWafMode = False
     kb.choices = AttribDict(keycheck=False)

lib/core/settings.py

@@ -20,7 +20,7 @@ from lib.core.enums import OS
 from thirdparty import six
 
 # sqlmap version (<major>.<minor>.<month>.<monthly commit>)
-VERSION = "1.10.8.16"
+VERSION = "1.10.8.17"
 TYPE = "dev" if VERSION.count('.') > 2 and VERSION.split('.')[-1] != '0' else "stable"
 TYPE_COLORS = {"dev": 33, "stable": 90, "pip": 34}
 VERSION_STRING = "sqlmap/%s#%s" % ('.'.join(VERSION.split('.')[:-1]) if VERSION.count('.') > 2 and VERSION.split('.')[-1] == '0' else VERSION, TYPE)

tests/test_kbchars.py

@@ -0,0 +1,78 @@
+#!/usr/bin/env python
+
+"""
+Copyright (c) 2006-2026 sqlmap developers (https://sqlmap.org)
+See the file 'LICENSE' for copying permission
+
+Invariants of the random markers in kb.chars (lib/core/option.py).
+
+These markers are drawn at random once per run, so a collision between two of them is a
+heisenbug: it corrupts extraction only in the fraction of runs that happen to draw the same
+value twice. Both collisions below were observed and are pinned here.
+
+  start/stop                 wrap the delimited output. Equal values make the output ambiguous
+                             (a two-row result then carries four identical markers instead of
+                             two pairs), so the parsing returns garbage or nothing at all.
+  at/space/dollar/hash_      stand in for '@', ' ', '$' and '#' inside a single chained REPLACE()
+                             (see the Oracle XMLType vectors in error_based.xml). Two equal
+                             markers make _errorReplaceChars() restore whichever character it
+                             tries first, so e.g. an '@' in an e-mail column comes back as ' '.
+
+The loop count is what matters: the smaller alphabet collides once every ~512 draws and the
+single-letter one every ~5, so a single sample proves nothing.
+
+stdlib unittest only (no pytest / no pip); works on Python 2.7 and 3.x.
+"""
+
+import os
+import sys
+import unittest
+
+sys.path.insert(0, os.path.dirname(os.path.abspath(__file__)))
+from _testutils import bootstrap
+bootstrap()
+
+from lib.core.data import kb
+from lib.core.option import _setKnowledgeBaseAttributes
+
+ROUNDS = 3000
+
+MARKERS = ("start", "stop", "at", "space", "dollar", "hash_")
+
+
+class KbCharsDistinctTest(unittest.TestCase):
+    """Re-draws the markers many times. kb is global, so it is snapshotted and put back -
+    leaking a fresh kb into the rest of the suite is how this bug got noticed in the first place."""
+
+    def setUp(self):
+        self._saved = dict(kb)
+
+    def tearDown(self):
+        kb.clear()
+        kb.update(self._saved)
+
+    def test_markers_never_collide(self):
+        for i in range(ROUNDS):
+            _setKnowledgeBaseAttributes()
+            drawn = [getattr(kb.chars, _) for _ in MARKERS]
+            self.assertEqual(len(set(drawn)), len(MARKERS),
+                             msg="colliding kb.chars markers on round %d: %s" % (i, dict(zip(MARKERS, drawn))))
+
+    def test_markers_keep_their_shape(self):
+        # the fix must not change the on-the-wire length of a payload
+        for _ in range(ROUNDS // 100):
+            _setKnowledgeBaseAttributes()
+            self.assertEqual(len(kb.chars.start), 5)
+            self.assertEqual(len(kb.chars.stop), 5)
+            for name in ("at", "space", "dollar", "hash_"):
+                self.assertEqual(len(getattr(kb.chars, name)), 3, msg=name)
+            self.assertEqual(len(kb.chars.delimiter), 6)
+
+    def test_kb_is_restored(self):
+        # guards the snapshot above, so a later failure elsewhere is never blamed on this file
+        _setKnowledgeBaseAttributes()
+        self.assertNotEqual(dict(kb).get("chars"), self._saved.get("chars"))
+
+
+if __name__ == "__main__":
+    unittest.main(verbosity=2)