diff --git a/htpclient/hashcat_cracker.py b/htpclient/hashcat_cracker.py index b01bb38..9e54975 100644 --- a/htpclient/hashcat_cracker.py +++ b/htpclient/hashcat_cracker.py @@ -13,7 +13,7 @@ from htpclient.hashcat_status import HashcatStatus from htpclient.initialize import Initialize from htpclient.jsonRequest import JsonRequest, os -from htpclient.helpers import send_error, update_files, kill_hashcat, get_bit, print_speed, get_rules_and_hl, get_wordlist, escape_ansi +from htpclient.helpers import send_error, update_files, kill_hashcat, get_bit, print_speed, get_rules_and_hl, get_wordlist, escape_ansi, format_error_detail from htpclient.dicts import * @@ -466,8 +466,14 @@ def measure_keyspace(self, task, chunk): logging.debug(f"CALL: {full_cmd}") output = subprocess.check_output(full_cmd, shell=True, cwd=self.cracker_path, stderr=subprocess.STDOUT) except subprocess.CalledProcessError as e: - logging.error("Error during keyspace measure: " + str(e) + " Output: " + output.decode(encoding='utf-8')) - send_error("Keyspace measure failed!", self.config.get_value('token'), task['taskId'], None) + # e.output carries hashcat's own stdout/stderr, the plain output variable is + # still empty here because the assignment above never completed. + detail = format_error_detail(e.output) + logging.error("Error during keyspace measure: " + str(e) + " Output: " + detail) + message = "Keyspace measure failed!" + if detail: + message += " " + detail + send_error(message, self.config.get_value('token'), task['taskId'], None) sleep(5) return False output = output.decode(encoding='utf-8').replace("\r\n", "\n").split("\n") @@ -494,10 +500,14 @@ def prince_keyspace(self, task, chunk): full_cmd = full_cmd.replace("/", '\\') try: logging.debug("CALL: " + full_cmd) - output = subprocess.check_output(full_cmd, shell=True, cwd="prince") - except subprocess.CalledProcessError: - logging.error("Error during PRINCE keyspace measure") - send_error("PRINCE keyspace measure failed!", self.config.get_value('token'), task['taskId'], None) + output = subprocess.check_output(full_cmd, shell=True, cwd="prince", stderr=subprocess.PIPE) + except subprocess.CalledProcessError as e: + detail = format_error_detail(e.stderr or e.output) + logging.error("Error during PRINCE keyspace measure" + (" Output: " + detail if detail else "")) + message = "PRINCE keyspace measure failed!" + if detail: + message += " " + detail + send_error(message, self.config.get_value('token'), task['taskId'], None) sleep(5) return False output = output.decode(encoding='utf-8').replace("\r\n", "\n").split("\n") @@ -539,10 +549,14 @@ def preprocessor_keyspace(self, task, chunk): try: logging.debug("CALL: " + full_cmd) - output = subprocess.check_output(full_cmd, shell=True, cwd=Path(preprocessors_path, str(task.get_task()['preprocessor']))) - except subprocess.CalledProcessError: - logging.error("Error during preprocessor keyspace measure") - send_error("Preprocessor keyspace measure failed!", self.config.get_value('token'), task.get_task()['taskId'], None) + output = subprocess.check_output(full_cmd, shell=True, cwd=Path(preprocessors_path, str(task.get_task()['preprocessor'])), stderr=subprocess.PIPE) + except subprocess.CalledProcessError as e: + detail = format_error_detail(e.stderr or e.output) + logging.error("Error during preprocessor keyspace measure" + (" Output: " + detail if detail else "")) + message = "Preprocessor keyspace measure failed!" + if detail: + message += " " + detail + send_error(message, self.config.get_value('token'), task.get_task()['taskId'], None) sleep(5) return False output = output.decode(encoding='utf-8').replace("\r\n", "\n").split("\n") diff --git a/htpclient/helpers.py b/htpclient/helpers.py index 2698cbe..e9a3bb4 100644 --- a/htpclient/helpers.py +++ b/htpclient/helpers.py @@ -57,6 +57,23 @@ def send_error(error, token, task_id, chunk_id): req.execute() +def format_error_detail(output, limit=2000): + # Turn the raw output of a failed cracker run into a short printable detail + # string that can be appended to a generic error message, so the server and + # the web UI show the real reason a run failed instead of only the generic + # text. See hashtopolis issue 746. The tail is kept because the actual error + # is usually the last thing the cracker prints, and it is capped so a runaway + # output cannot blow past the server's error column. + if output is None: + return '' + if isinstance(output, bytes): + output = output.decode('utf-8', errors='replace') + output = escape_ansi(output.replace("\r\n", "\n")).strip() + if len(output) > limit: + output = output[-limit:] + return output + + def file_get_contents(filename): with open(filename) as f: return f.read() diff --git a/tests/test_error_detail.py b/tests/test_error_detail.py new file mode 100644 index 0000000..277d1af --- /dev/null +++ b/tests/test_error_detail.py @@ -0,0 +1,105 @@ +import subprocess +import unittest +from unittest import mock + +from htpclient.helpers import format_error_detail +from htpclient.hashcat_cracker import HashcatCracker + + +class TestFormatErrorDetail(unittest.TestCase): + def test_none_is_empty(self): + self.assertEqual(format_error_detail(None), '') + + def test_empty_bytes_is_empty(self): + self.assertEqual(format_error_detail(b''), '') + + def test_decodes_bytes(self): + self.assertEqual(format_error_detail(b'plain error'), 'plain error') + + def test_strips_ansi_and_normalises_newlines(self): + # A colour-wrapped two line message, CRLF terminated, trailing spaces. + raw = b'\x1b[31mSeparator unmatched\x1b[0m\r\nsecond line ' + self.assertEqual(format_error_detail(raw), 'Separator unmatched\nsecond line') + + def test_invalid_utf8_does_not_raise(self): + # \xff is not valid UTF-8, so it decodes to U+FFFD (chr(0xfffd)). + self.assertEqual(format_error_detail(b'bad \xff byte'), 'bad ' + chr(0xfffd) + ' byte') + + def test_keeps_the_tail_when_over_the_limit(self): + # The real error is usually the last thing printed, so the tail is kept. + out = ('noise\n' * 1000) + 'THE REAL ERROR' + detail = format_error_detail(out, limit=40) + self.assertEqual(len(detail), 40) + self.assertTrue(detail.endswith('THE REAL ERROR')) + + def test_default_limit_is_2000(self): + self.assertEqual(len(format_error_detail('x' * 3000)), 2000) + + +class TestKeyspaceErrorDetail(unittest.TestCase): + """The keyspace measure failure path must send hashcat's own output to the + server, not only the generic 'Keyspace measure failed!' text (issue 746).""" + + def _make_cracker(self): + cracker = HashcatCracker.__new__(HashcatCracker) + cracker.callPath = './hashcat.bin' + cracker.cracker_path = '.' + cracker.config = mock.MagicMock() + cracker.config.get_value.return_value = 'devtoken' + return cracker + + @mock.patch('htpclient.hashcat_cracker.sleep', return_value=None) + @mock.patch('htpclient.hashcat_cracker.send_error') + @mock.patch('htpclient.hashcat_cracker.update_files', return_value=' #HL# -a3 ?l?l?l?l ') + @mock.patch('htpclient.hashcat_cracker.subprocess.check_output') + def test_failure_propagates_detail(self, mock_check, mock_update, mock_send, mock_sleep): + raw = b'\x1b[31mHashfile on line 1: Separator unmatched\x1b[0m\r\n' + mock_check.side_effect = subprocess.CalledProcessError(255, 'cmd', output=raw) + + task = mock.MagicMock() + task.get_task.return_value = { + 'attackcmd': '#HL# -a3 ?l?l?l?l', + 'hashlistAlias': '#HL#', + 'cmdpars': '', + 'taskId': 7, + } + chunk = mock.MagicMock() + + result = self._make_cracker().measure_keyspace(task, chunk) + + self.assertFalse(result) + chunk.send_keyspace.assert_not_called() + mock_send.assert_called_once() + + message, token, task_id, chunk_id = mock_send.call_args[0] + self.assertIn('Keyspace measure failed!', message) + self.assertIn('Separator unmatched', message) + self.assertNotIn('\x1b', message) + self.assertEqual(task_id, 7) + self.assertIsNone(chunk_id) + + @mock.patch('htpclient.hashcat_cracker.sleep', return_value=None) + @mock.patch('htpclient.hashcat_cracker.send_error') + @mock.patch('htpclient.hashcat_cracker.update_files', return_value=' #HL# -a3 ?l?l?l?l ') + @mock.patch('htpclient.hashcat_cracker.subprocess.check_output') + def test_failure_without_output_sends_generic_message(self, mock_check, mock_update, mock_send, mock_sleep): + # A crash that printed nothing must still send the generic message, with + # no dangling separator from an empty detail. + mock_check.side_effect = subprocess.CalledProcessError(255, 'cmd', output=b'') + + task = mock.MagicMock() + task.get_task.return_value = { + 'attackcmd': '#HL# -a3 ?l?l?l?l', + 'hashlistAlias': '#HL#', + 'cmdpars': '', + 'taskId': 7, + } + + self._make_cracker().measure_keyspace(task, mock.MagicMock()) + + message = mock_send.call_args[0][0] + self.assertEqual(message, 'Keyspace measure failed!') + + +if __name__ == '__main__': + unittest.main()