Skip to content

Commit 0cf2f6d

Browse files
Fix REST API multi-target scans returning only the last target's results (#2503) (#6146)
A scan covering multiple targets inside a single API task (crawl, '-m' bulk file, forms) ran every target through one StdDbOut keyed by the single taskid, so _storeData()'s COMPLETE branch deleted each earlier target's TARGET/TECHNIQUES rows - /scan/<id>/data returned only the last target's results. Mirror the --report-json fix for #6123: controller.start() now sets kb.reportTaskId per target for API scans too, StdDbOut.write() keys data rows '<taskid>-<n>' per target, and scan_data() folds the per-target rows back into the task's flat result in target order. Single-target scans keep the exact same response shape. Regression tests in tests/test_api.py (TestStdDbOutMultiTarget, TestScanDataMultiTarget).
1 parent c94f6d0 commit 0cf2f6d

3 files changed

Lines changed: 104 additions & 4 deletions

File tree

‎lib/controller/controller.py‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -376,9 +376,10 @@ def start():
376376
for targetUrl, targetMethod, targetData, targetCookie, targetHeaders in kb.targets:
377377
targetCount += 1
378378

379-
# --report-json: give each target its own taskid in the shared collector, so a multi-target
380-
# run (e.g. '-m' bulk file) doesn't have later targets overwrite earlier ones (see _reportData)
381-
if conf.reportJson:
379+
# --report-json / REST API: give each target its own taskid in the shared collector, so a
380+
# multi-target run (e.g. '-m' bulk file) doesn't have later targets overwrite earlier ones
381+
# (see _reportData for the report collector and StdDbOut.write for the REST API IPC database)
382+
if conf.reportJson or conf.api:
382383
kb.reportTaskId = targetCount
383384

384385
try:

‎lib/utils/api.py‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,17 @@ def __init__(self, taskid, messagetype="stdout"):
444444

445445
def write(self, value, status=CONTENT_STATUS.IN_PROGRESS, content_type=None):
446446
if self.messagetype == "stdout":
447-
_storeData(conf.databaseCursor, self.taskid, value, status, content_type)
447+
# A multi-target scan (e.g. crawling or a '-m' bulk file inside a single API task) runs
448+
# every target through this one StdDbOut instance. Key each target's data rows by
449+
# kb.reportTaskId (set per target in controller.start()) so that a later target's
450+
# COMPLETE row cannot delete an earlier target's rows of the same content_type in
451+
# _storeData() - the REST API counterpart of the --report-json fix for #6123, and the
452+
# reason a crawl via sqlmapapi used to return only the last target's results (#2503).
453+
# scan_data() folds the per-target rows back into the task's flat result.
454+
taskid = self.taskid
455+
if kb.get("reportTaskId"):
456+
taskid = "%s-%s" % (self.taskid, kb.reportTaskId)
457+
_storeData(conf.databaseCursor, taskid, value, status, content_type)
448458
else:
449459
conf.databaseCursor.execute("INSERT INTO errors VALUES(NULL, ?, ?)", (self.taskid, str(value) if value else ""))
450460

@@ -818,6 +828,18 @@ def scan_data(taskid):
818828
# Read all data and error messages from the IPC database (shared assembler - same output as --report-json)
819829
result = _assembleData(DataStore.current_db, taskid)
820830

831+
# Multi-target scans store each target's data rows under a '<taskid>-<n>' key (see
832+
# StdDbOut.write); fold them into this task's flat result, in target order. Real taskids
833+
# are dash-less hex strings, so a '<taskid>-' prefix can only match this task's own rows.
834+
prefix = "%s-" % taskid
835+
subtaskids = []
836+
for row in DataStore.current_db.execute("SELECT DISTINCT taskid FROM data"):
837+
stored = str(row[0])
838+
if stored.startswith(prefix) and stored[len(prefix):].isdigit():
839+
subtaskids.append(stored)
840+
for subtaskid in sorted(subtaskids, key=lambda _: int(_.rsplit("-", 1)[1])):
841+
result["data"].extend(_assembleData(DataStore.current_db, subtaskid)["data"])
842+
821843
logger.debug("(%s) Retrieved scan data and error messages" % taskid)
822844
return jsonize(result)
823845

‎tests/test_api.py‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030

3131
import lib.utils.api as api
3232
from lib.core.data import conf
33+
from lib.core.data import kb
3334
from lib.core.convert import encodeBase64
3435
from lib.core.enums import CONTENT_STATUS, CONTENT_TYPE
3536
from thirdparty.bottle.bottle import default_app
@@ -308,6 +309,62 @@ def test_logrecorder_emit_stores_log(self):
308309
self.assertEqual(rows[0][2], "hello world")
309310

310311

312+
class TestStdDbOutMultiTarget(unittest.TestCase):
313+
"""
314+
Regression coverage for issue #2503: a multi-target scan (crawl or '-m' bulk
315+
file) inside a single REST API task runs every target through one StdDbOut
316+
instance. StdDbOut.write() keys each target's data rows by kb.reportTaskId
317+
('<taskid>-<n>'), so a later target's COMPLETE row cannot delete an earlier
318+
target's rows of the same content_type in _storeData() - mirroring the
319+
--report-json fix for #6123 (see test_report.py's TestMultiTargetTaskId).
320+
"""
321+
322+
def setUp(self):
323+
self.db = api.Database(":memory:")
324+
self.db.connect("client")
325+
self.db.init()
326+
self._saved_stdout = sys.stdout
327+
self._saved_cursor = conf.get("databaseCursor")
328+
self._saved_partRun = kb.get("partRun")
329+
self._had_reportTaskId = "reportTaskId" in kb
330+
self._saved_reportTaskId = kb.get("reportTaskId")
331+
conf.databaseCursor = self.db
332+
kb.partRun = None
333+
334+
def tearDown(self):
335+
sys.stdout = self._saved_stdout
336+
conf.databaseCursor = self._saved_cursor
337+
kb.partRun = self._saved_partRun
338+
if self._had_reportTaskId:
339+
kb.reportTaskId = self._saved_reportTaskId
340+
elif "reportTaskId" in kb:
341+
del kb.reportTaskId
342+
self.db.disconnect()
343+
344+
def test_second_target_does_not_overwrite_first(self):
345+
std = api.StdDbOut("t1", messagetype="stdout")
346+
sys.stdout = self._saved_stdout # StdDbOut hijacked it; call write() directly
347+
348+
kb.reportTaskId = 1
349+
std.write({"url": "http://host1/?id=1"}, status=CONTENT_STATUS.COMPLETE, content_type=CONTENT_TYPE.TARGET)
350+
kb.reportTaskId = 2
351+
std.write({"url": "http://host2/?id=1"}, status=CONTENT_STATUS.COMPLETE, content_type=CONTENT_TYPE.TARGET)
352+
353+
rows = self.db.execute("SELECT taskid FROM data ORDER BY id")
354+
self.assertEqual([row[0] for row in rows], ["t1-1", "t1-2"]) # neither target's row was deleted
355+
356+
def test_single_target_without_report_task_id_keeps_plain_taskid(self):
357+
if "reportTaskId" in kb:
358+
del kb.reportTaskId
359+
std = api.StdDbOut("t1", messagetype="stdout")
360+
sys.stdout = self._saved_stdout
361+
362+
std.write({"url": "http://host1/?id=1"}, status=CONTENT_STATUS.COMPLETE, content_type=CONTENT_TYPE.TARGET)
363+
364+
rows = self.db.execute("SELECT taskid FROM data")
365+
self.assertEqual([row[0] for row in rows], ["t1"])
366+
367+
311368
# ---------------------------------------------------------------------------
312369
# HTTP routes (WSGI test client)
313370
# ---------------------------------------------------------------------------
@@ -485,6 +542,26 @@ def test_scan_kill_invalid_when_not_running(self):
485542
self.assertFalse(parsed["success"])
486543

487544

545+
class TestScanDataMultiTarget(_ApiServerCase):
546+
"""scan_data() folds the per-target '<taskid>-<n>' rows (see StdDbOut.write) back into the task's flat result, in target order."""
547+
548+
def test_data_merges_per_target_rows(self):
549+
taskid = self._new_task()
550+
api._storeData(self.db, "%s-2" % taskid, {"url": "http://host2/?id=1"}, CONTENT_STATUS.COMPLETE, CONTENT_TYPE.TARGET)
551+
api._storeData(self.db, "%s-1" % taskid, {"url": "http://host1/?id=1"}, CONTENT_STATUS.COMPLETE, CONTENT_TYPE.TARGET)
552+
code, parsed, _ = _wsgi_call("GET", "/scan/%s/data" % taskid)
553+
self.assertEqual(code, 200)
554+
self.assertTrue(parsed["success"])
555+
self.assertEqual([d["value"]["url"] for d in parsed["data"]], ["http://host1/?id=1", "http://host2/?id=1"])
556+
557+
def test_data_ignores_other_tasks_rows(self):
558+
taskid = self._new_task()
559+
api._storeData(self.db, "othertask-1", {"url": "http://other/?id=1"}, CONTENT_STATUS.COMPLETE, CONTENT_TYPE.TARGET)
560+
code, parsed, _ = _wsgi_call("GET", "/scan/%s/data" % taskid)
561+
self.assertTrue(parsed["success"])
562+
self.assertEqual(parsed["data"], [])
563+
564+
488565
class TestScanStart(_ApiServerCase):
489566
"""scan_start, with the subprocess-spawning seam (engine_start) monkeypatched."""
490567

0 commit comments

Comments
 (0)