diff options
| -rw-r--r-- | docs/PROJECTS.md | 19 | ||||
| -rw-r--r-- | docs/TEXTUAL_NOTES.md | 30 | ||||
| -rw-r--r-- | idatui/app.py | 257 | ||||
| -rw-r--r-- | idatui/project.py | 23 | ||||
| -rw-r--r-- | idatui/worker.py | 9 | ||||
| -rw-r--r-- | tests/test_blob_ui.py | 106 | ||||
| -rw-r--r-- | tests/test_scenarios.py | 25 |
7 files changed, 454 insertions, 15 deletions
diff --git a/docs/PROJECTS.md b/docs/PROJECTS.md index 88ce2f6..10274ab 100644 --- a/docs/PROJECTS.md +++ b/docs/PROJECTS.md @@ -297,6 +297,25 @@ conversion happens in `BinaryRef.load_args`, and a base that isn't 16-byte aligned is rejected rather than silently landing somewhere else. `ida_args` passes anything else through untouched. +### When analysis finds nothing + +Zero functions is what a raw image described wrongly looks like — right file, +wrong architecture, and IDA has no complaint to make about it. The app used to +fall through to the symbol picker, which had nothing to show, so you got empty +panes and a status reading "functions still loading…" long after loading had +finished. + +Now it opens the listing at the start of the image (the bytes are there even when +no code was recognised) and the status bar carries the diagnosis for as long as +it's true: + + seg000 @ 0x0 [listing] — no functions: wrong processor/base? Ctrl+L to reload + +`Ctrl+L` re-asks. The database IDA already built has the old processor and base +baked into it and wins over any switches, so reloading means deleting it — hence +the confirmation, which tells you what you'd lose (and, in this case, that you'd +lose nothing). + Two things worth knowing: * The options apply to the **first** open only. Afterwards the `.i64` records how diff --git a/docs/TEXTUAL_NOTES.md b/docs/TEXTUAL_NOTES.md index 4bf7fcb..059b47c 100644 --- a/docs/TEXTUAL_NOTES.md +++ b/docs/TEXTUAL_NOTES.md @@ -80,3 +80,33 @@ Hard-won Textual behaviour and the patterns this app relies on. Pairs with failures. Re-run on a warm session before trusting a regression. - Edits mutate the `.i64` — revert renames (via API) for idempotency; use a PID-unique temp name to dodge collisions from a prior crashed run. + + +## A priority app binding fires even under a modal + +`Binding("tab,shift+tab", "toggle_view", priority=True)` on the App runs before +the focus chain, so Tab never reached ANY dialog — nothing in a modal could be +tabbed to, in this app, ever. It looked like a bug in one dialog (the load +options address field was unreachable) and was actually app-wide. + +The fix is in the action, not the binding: if a modal is up, hand the key back. + +```python +def action_toggle_view(self): + if self.screen is not self.screen_stack[0]: + self.screen.focus_next() + return +``` + +Related: DOM order is rarely the tab order you want. In a dialog with a filter +box, an option list and a second field, `focus_next()` stops at the list — which +is arrow-driven and has nothing to type — so text meant for the second field +lands in the filter. Override `focus_next`/`focus_previous` on the screen to +cycle the fields people actually type into. + +## A modal can outgrow the terminal and clip its own controls + +`#pal-list { max-height: 24 }` plus a 21-row list pushed the address field and +help line off the bottom of the screen. Nothing errors; the field is simply not +there, which reads to the user as "Tab does nothing". Cap the scrollable part +per-dialog so the controls below it are always visible. diff --git a/idatui/app.py b/idatui/app.py index f861f42..f1b4dec 100644 --- a/idatui/app.py +++ b/idatui/app.py @@ -2566,6 +2566,24 @@ class LoadOptionsScreen(ModalScreen): self._apply("") self.query_one("#pal-input", Input).focus() + def focus_next(self, selector="*"): # type: ignore[override] + """Tab moves between the two things you TYPE into. + + DOM order would stop at the option list on the way, which is + arrow-driven and has nothing to type — and the address you meant to + enter goes into whichever box happened to have focus. Typing an address + into the processor filter is then taken as a processor name, IDA rejects + it, and the open fails; that is a bad enough outcome to be worth + overriding Tab for. + """ + inp = self.query_one("#pal-input", Input) + base = self.query_one("#load-base", Input) + (inp if self.focused is base else base).focus() + return self.focused + + def focus_previous(self, selector="*"): # type: ignore[override] + return self.focus_next() + def on_input_changed(self, event: Input.Changed) -> None: event.stop() if event.input.id == "pal-input": @@ -2739,13 +2757,16 @@ class ConfirmScreen(ModalScreen): Binding("escape,n", "cancel", "Cancel"), ] - def __init__(self, message: str) -> None: + def __init__(self, message: str, note: str = "") -> None: super().__init__() self._message = message + self._note = note def compose(self) -> ComposeResult: with Vertical(id="confirm-box"): - yield Static(self._message, id="confirm-msg") + yield Static(self._message, id="confirm-msg", markup=False) + if self._note: + yield Static(self._note, id="confirm-note", markup=False) yield Static("[Enter/y] confirm [Esc/n] cancel", id="confirm-help") def action_confirm(self) -> None: @@ -3238,8 +3259,13 @@ class IdaTui(App): #pal-input { border: none; height: 1; margin: 0 1; background: $panel; color: $text; } #pal-list { height: auto; max-height: 24; } #load-note { height: 2; padding: 1 1 0 1; color: $text-muted; } + /* Cap the processor list so the ADDRESS FIELD is always on screen: with the + palette default (24) the box outgrew the terminal and the field you need + was clipped off the bottom, which read as "Tab does nothing". */ + LoadOptionsScreen #pal-list { max-height: 12; } #load-base { border: none; height: 1; margin: 1 1 0 1; background: $panel; color: $text; } #load-help { height: 1; padding: 0 1; color: $text-muted; } + #confirm-note { height: auto; padding: 0 1; color: $text-muted; } StructEditor { align: center middle; } #se-box { width: 90%; height: 84%; border: thick $accent; background: $panel; } #se-panes { height: 1fr; } @@ -3276,6 +3302,7 @@ class IdaTui(App): Binding("s", "toggle_split", "Split", show=False), Binding("quotation_mark,shift+f12", "strings", "Strings", show=False), Binding("ctrl+o", "switch_binary", "Binaries", show=False), + Binding("ctrl+l", "load_options", "Reload as…", show=False), Binding("f1", "help", "Keys", show=False), Binding("g", "goto", "Goto"), Binding("slash", "filter", "Filter", show=False), @@ -3298,6 +3325,9 @@ class IdaTui(App): self._pending_restore = None # entry to reopen after a switch self._goto_after_switch = None # cross-binary search hit to land on self._hops: list[str] = [] # binaries a navigation crossed FROM + self._load_for_label = None # project binary the dialog is for + self._no_functions = False # analysis produced nothing at all + self._pending_switch = None # switch waiting on that answer self._nav_seq = 0 # bumped per navigation; drops stale ones # None = teardown wasn't an explicit quit (crash/kill): save defensively. # False = the user chose discard, or we already saved on the way out. @@ -3412,8 +3442,13 @@ class IdaTui(App): # A file no loader recognises has to be described before it can be # opened, so ask BEFORE the worker starts — once IDA has made a database # the answer is baked in and changing it means deleting the .i64. - if self._should_ask_load_options(): - self._ask_load_options() + if self._project is not None: + ref = self._pending_load_ref() + if ref is not None: + self._ask_load_options(ref.source, label=ref.label) + return + elif self._should_ask_load_options(): + self._ask_load_options(self._open_path) return # Show a loading overlay immediately so a slow open/analysis (big binary) # isn't just dead air behind empty panes; dismissed once we land. @@ -3429,8 +3464,8 @@ class IdaTui(App): re-passing switches fails the open); or the file is a format IDA recognises, which is nearly always. """ - if self._project is not None or not self._open_path: - return False # project mode carries per-binary options already + if not self._open_path: + return False if self._load_args: return False from .formats import needs_load_options @@ -3439,21 +3474,151 @@ class IdaTui(App): return False return needs_load_options(self._open_path) - def _ask_load_options(self) -> None: + def action_load_options(self) -> None: + """Ctrl+L: re-open this binary with different load options. + + The database IDA already built has the old processor and base baked into + it and takes precedence over any switches, so re-loading means throwing + it away. That destroys names and comments, hence the confirmation — but + for the case this exists for (a blob loaded as the wrong architecture, + which analysed to nothing) there is nothing to lose and no other way + forward. + """ + if not self._can_reload(): + self._status("nothing to reload") + return + n = len(self._func_index) if self._func_index else 0 + note = ("this image has no functions, so nothing is lost" + if n == 0 else + f"discards the database for this binary \u2014 {n} functions, " + f"plus any names and comments you've added") + self.push_screen(ConfirmScreen("Reload with different options?", note), + self._on_reload_confirmed) + + def _on_reload_confirmed(self, yes) -> None: # type: ignore[no-untyped-def] + if not yes: + return + path, label = self._open_path, None + if self._project is not None and self._binary is not None: + ref = self._project.by_label(self._binary) + if ref is not None: + path, label = ref.source, ref.label + # Drop the worker first: it holds the database open, and the .i64 can't + # be removed (or rebuilt) underneath a live one. + self._release_worker() + self._drop_database() + self._reset_for_reload() + self._load_args = "" + if label is not None and self._project is not None: + self._project.set_load(label, processor="", base=0) + i = self._project._refs.index(self._project.by_label(label)) + self._project._entries[i].pop("processor", None) + self._project._entries[i].pop("base", None) + self._project.save() + self._pending_switch = None + self._ask_load_options(path, label=label) + + def _release_worker(self) -> None: + if self._pool is not None and self._binary is not None: + try: + self._pool.evict(self._binary, save=False) + except Exception: # noqa: BLE001 + pass + elif self.client is not None: + try: + self.client.close() + except Exception: # noqa: BLE001 + pass + self.client = None + self.program = None + + def _drop_database(self) -> None: + """Remove the .i64 (and any unpacked scratch) so the next open re-reads + the raw image with new options.""" + base = self._open_path + if self._project is not None and self._binary is not None: + ref = self._project.by_label(self._binary) + if ref is not None: + base = ref.staged + if not base: + return + for suffix in (".i64", ".id0", ".id1", ".id2", ".nam", ".til"): + for cand in (base + suffix, os.path.splitext(base)[0] + suffix): + try: + os.remove(cand) + except OSError: + pass + + def _reset_for_reload(self) -> None: + self._no_functions = False + self._func_index = None + self._cur = None + self._nav = [] + self._did_auto_land = False + self._pending_restore = None + self._split = False + self.query_one(DecompView).loaded_ea = None + + def _retry_load_options(self) -> None: + """Re-ask after IDA refused what we told it.""" + path, label = self._open_path, None + if self._project is not None and self._binary is not None: + ref = self._project.by_label(self._binary) + if ref is not None: + path, label = ref.source, ref.label + # Clear the rejected answer or _pending_load_ref would see the + # binary as already described and never ask again. + self._project.set_load(label, processor="", base=0) + self._project._entries[self._project._refs.index(ref)].pop( + "processor", None) + self._project.save() + self._load_args = "" + if path: + self._status("those load options were rejected \u2014 try again") + self._ask_load_options(path, label=label) + + def _pending_load_ref(self, label: str | None = None): # type: ignore[no-untyped-def] + """The project binary about to be opened, if it needs describing. + + Checked against the SOURCE: staging may not have happened yet, and the + question is about the bytes, not where they were copied to. + """ + if self._project is None: + return None + label = label or self._binary or self._project.refs[0].label + ref = self._project.by_label(label) + if ref is None or ref.load_args: + return None + if os.path.exists(ref.db) or os.path.exists( + os.path.splitext(ref.staged)[0] + ".i64"): + return None # already analysed: the .i64 records how + from .formats import needs_load_options + return ref if needs_load_options(ref.source) else None + + def _ask_load_options(self, path: str, label: str | None = None) -> None: try: - size = os.path.getsize(self._open_path) + size = os.path.getsize(path) except OSError: size = 0 - self.push_screen(LoadOptionsScreen(self._open_path, size), - self._on_load_options) + self._load_for_label = label + self.push_screen(LoadOptionsScreen(path, size), self._on_load_options) def _on_load_options(self, choice) -> None: # type: ignore[no-untyped-def] from .formats import load_args choice = choice or {} - if choice.get("processor"): - self._load_args = load_args(choice["processor"], choice.get("base", 0)) - self._status(f"loading as {choice['processor']} " - f"@ {choice.get('base', 0):#x}") + label, self._load_for_label = self._load_for_label, None + proc, base = choice.get("processor", ""), int(choice.get("base", 0) or 0) + if proc: + if label is not None and self._project is not None: + # Persist it: the answer belongs to the binary, not to this run. + self._project.set_load(label, proc, base) + else: + self._load_args = load_args(proc, base) + self._status(f"loading as {proc} @ {base:#x}") + if self._pending_switch is not None: + label2, self._pending_switch = self._pending_switch, None + self._switch_binary(label2) + return self._loading_screen = LoadingScreen(self._loading_title()) self.push_screen(self._loading_screen) self._connect() @@ -3488,6 +3653,11 @@ class IdaTui(App): def _status(self, text: str) -> None: if self._binary: # project mode: always say which binary you're in text = f"[{self._binary}] {text}" + # An image with no functions at all is nearly always a blob described + # wrongly, and that stays true as you scroll around — so it belongs in + # the status bar, not in a one-off message the next write clobbers. + if self._no_functions: + text += " \u2014 no functions: wrong processor/base? Ctrl+L to reload" try: self.query_one("#status", Static).update(text) except Exception: # noqa: BLE001 -- status bar transiently unavailable @@ -3600,6 +3770,12 @@ class IdaTui(App): except Exception as e: # noqa: BLE001 self.app.call_from_thread(self._status, f"connect failed: {e}") self.app.call_from_thread(self._dismiss_loading) + # A load we described ourselves and IDA refused: offer the dialog + # again rather than leaving an empty app with an error in the status + # bar. Getting the processor wrong is an ordinary mistake and should + # cost one more keypress, not a restart. + if "load options" in str(e): + self.app.call_from_thread(self._retry_load_options) return self.client = client self.program = program @@ -3757,8 +3933,42 @@ class IdaTui(App): fn = self._entry_func() if fn is not None: self._open_function(fn.addr, fn.name) - else: + elif len(self._func_index): self.action_symbols() + else: + self._land_without_functions() + + def _land_without_functions(self) -> None: + """Analysis found nothing. Show the bytes and say so. + + Falling through to the symbol picker here left two empty panes and + "functions still loading…" — which is a lie, loading had finished. There + is always something to look at: the segments exist even when IDA + recognised no code in them, so open the listing at the start of the image. + + Zero functions is also the signal that a blob was described wrongly. It's + exactly what a good image loaded as the wrong processor looks like, so + the status says so rather than leaving you to guess. + """ + start = None + try: + regions = self.program.file_regions() + if regions: + start = regions[0][0] + except Exception: # noqa: BLE001 + pass + self._no_functions = self._can_reload() + if start is None: + self._status("no functions and no segments \u2014 nothing to show") + return + self._open_at(start, self.program.section_of(start) or "image", + cursor=0, push=True, is_region=True) + + def _can_reload(self) -> bool: + """Whether we're able to re-open this binary with different options.""" + if self._project is not None and self._binary is not None: + return True + return bool(self._open_path) def _entry_func(self) -> Func | None: """The best startup landing function (exact-name match against @@ -3993,6 +4203,13 @@ class IdaTui(App): self._switch_binary(label) def _switch_binary(self, label: str) -> None: + # A blob nobody has described yet has to be described before its worker + # opens it — same as at boot, just reached by switching instead. + ref = self._pending_load_ref(label) + if ref is not None and self._load_for_label is None: + self._pending_switch = label + self._ask_load_options(ref.source, label=label) + return # Snapshot what we're leaving so coming back restores the view, then let # the pool hand us a worker (spawning + evicting as the budget dictates). if self._binary is not None: @@ -4134,6 +4351,16 @@ class IdaTui(App): def action_toggle_view(self) -> None: """Tab: switch the code pane between disassembly and pseudocode (or leave the hex view back to the preferred code view).""" + # Tab is a PRIORITY app binding, so it fires even while a modal is up and + # nothing inside a dialog could ever be tabbed to. Hand it back to the + # dialog: this is the only reason the load dialog's address field was + # unreachable, and it was broken the same way in every other modal. + if self.screen is not self.screen_stack[0]: + try: + self.screen.focus_next() + except Exception: # noqa: BLE001 -- screen with nothing focusable + pass + return if self._cur is None: return if self._split: diff --git a/idatui/project.py b/idatui/project.py index 8f2674d..e2fc542 100644 --- a/idatui/project.py +++ b/idatui/project.py @@ -239,6 +239,29 @@ class Project: def refs(self) -> tuple[BinaryRef, ...]: return self._refs + def set_load(self, label: str, processor: str = "", base: int = 0, + ida_args: str = "") -> BinaryRef | None: + """Record how ``label`` should be loaded, and persist it. + + Answered once: the dialog that asks writes the answer here, so reopening + the project doesn't ask again — and neither does adding the same blob to + another project, since it travels with the entry. + """ + ref = self.by_label(label) + if ref is None: + return None + i = self._refs.index(ref) + e = self._entries[i] + if processor: + e["processor"] = processor + if base: + e["base"] = int(base) + if ida_args: + e["ida_args"] = ida_args + self._refs = self._build_refs() + self.save() + return self._refs[i] + def by_label(self, label: str) -> BinaryRef | None: return next((r for r in self._refs if r.label == label), None) diff --git a/idatui/worker.py b/idatui/worker.py index 26f0816..a4e3509 100644 --- a/idatui/worker.py +++ b/idatui/worker.py @@ -111,6 +111,15 @@ def _open_and_register(binpath: str, load_args: str = ""): args = None if idapro.open_database(binpath, run_auto_analysis=True, args=args): # nonzero == failure + if args: + # With load switches in play they are the likeliest culprit by far: + # IDA refuses an unknown -p name with no diagnostic of its own, so + # saying "the database is locked" here sends people hunting a + # problem they don't have. + raise RuntimeError( + f"failed to open {binpath} with load options {args!r}: IDA " + f"rejected them \u2014 an unknown processor name is the usual " + f"cause (see tools/verify_procs.py for the valid ones)") raise RuntimeError( f"failed to open {binpath}: the .i64 is likely held by a running " f"ida-mcp worker (try: pkill -f idalib) or wedged from a crash " diff --git a/tests/test_blob_ui.py b/tests/test_blob_ui.py new file mode 100644 index 0000000..e8d2533 --- /dev/null +++ b/tests/test_blob_ui.py @@ -0,0 +1,106 @@ +#!/usr/bin/env python3 +"""A blob that analyses to NOTHING must still be usable. + +Zero functions is the normal outcome for a raw image described wrongly — and it +used to leave two empty panes and a status that said "functions still loading…", +which was a lie: loading had finished, there was simply nothing to land on. + +Needs IDA (spawns a real worker). ~40s. +""" +import asyncio +import os +import sys +import tempfile + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from textual.widgets import Static # noqa: E402 + +from idatui.app import ConfirmScreen, IdaTui, ListingView # noqa: E402 + +PASS = FAIL = 0 + + +def check(name, ok, detail=""): + global PASS, FAIL + if ok: + PASS += 1 + print(f" ok {name}") + else: + FAIL += 1 + print(f" FAIL {name} {detail}") + + +async def wait(pred, pilot, t=240.0): + for _ in range(int(t / 0.05)): + await pilot.pause(0.05) + try: + if pred(): + return True + except Exception: # noqa: BLE001 + pass + return False + + +async def run() -> int: + with tempfile.TemporaryDirectory() as tmp: + blob = os.path.join(tmp, "rnd.bin") + with open(blob, "wb") as f: + f.write(os.urandom(64 * 1024)) # random: IDA will find no code + + # Skip the dialog by answering up front; this test is about what + # happens AFTER a described blob turns out to contain nothing. + app = IdaTui(open_path=blob, keepalive=False, load_args="-parm -b400") + async with app.run_test(size=(140, 44)) as pilot: + ok = await wait(lambda: app._func_index is not None + and app._func_index.complete, pilot) + check("a random blob still finishes loading", ok) + check("and really has no functions", len(app._func_index) == 0, + f"n={len(app._func_index)}") + + landed = await wait(lambda: app._cur is not None, pilot, 60) + check("it lands somewhere instead of leaving empty panes", landed, + f"cur={app._cur}") + lst = app.query_one(ListingView) + check("the listing actually has rows to show", + lst.total > 0, f"total={lst.total}") + check("landed at the image base", app._cur is not None + and app._cur.ea == 0x4000, f"{app._cur.ea if app._cur else None:#x}") + + status = str(app.query_one("#status", Static).render()) + check("the status says there are no functions (not 'still loading')", + "no functions" in status and "still loading" not in status, + status[:90]) + check("and points at the likely cause", + "processor" in status and "Ctrl+L" in status, status[:90]) + + # The hint is a property of the database, so it must survive moving + # around — an earlier version wrote it once and the next status + # write erased it. + lst.focus() + await pilot.press("down") + await pilot.press("down") + await pilot.pause(0.3) + status2 = str(app.query_one("#status", Static).render()) + check("the hint survives navigating", "no functions" in status2, + status2[:90]) + + await pilot.press("ctrl+l") + opened = await wait(lambda: isinstance(app.screen, ConfirmScreen), pilot, 20) + check("Ctrl+L offers to reload with different options", opened, + f"screen={type(app.screen).__name__}") + if opened: + note = str(app.screen.query_one("#confirm-note", Static).render()) + check("and says nothing is lost when there's nothing to lose", + "nothing is lost" in note, note[:70]) + await pilot.press("escape") + await pilot.pause(0.3) + check("declining leaves the binary open", + not isinstance(app.screen, ConfirmScreen) and app._cur is not None) + + print(f"\n{PASS} passed, {FAIL} failed") + return 1 if FAIL else 0 + + +if __name__ == "__main__": + raise SystemExit(asyncio.run(run())) diff --git a/tests/test_scenarios.py b/tests/test_scenarios.py index ee2f77b..6ccbf0d 100644 --- a/tests/test_scenarios.py +++ b/tests/test_scenarios.py @@ -369,6 +369,31 @@ async def s_load_options(c: Ctx): c.check("dialog output converts a base to paragraphs", load_args("arm", 0x8000000) == "-parm -b800000") + # Tab is a PRIORITY app binding (disasm<->pseudocode), so it fired even with + # a modal up and nothing in a dialog could be tabbed to. That is why the load + # dialog's address field was unreachable — and it was broken in every other + # modal too. + from idatui.app import LoadOptionsScreen + app.push_screen(LoadOptionsScreen("/tmp/probe.bin", 1234)) + await c.wait(lambda: isinstance(app.screen, LoadOptionsScreen), 10) + sc = app.screen + first = app.focused + await c.press("tab") + await c.pause(0.2) + c.check("Tab moves focus inside a modal instead of toggling the view", + app.focused is not first and isinstance(app.screen, LoadOptionsScreen), + f"focus={getattr(app.focused, 'id', None)}") + c.check("Tab in the load dialog lands on the address field", + getattr(app.focused, "id", None) == "load-base", + f"focus={getattr(app.focused, 'id', None)}") + await c.press("tab") + await c.pause(0.2) + c.check("Tab again returns to the processor filter", + getattr(app.focused, "id", None) == "pal-input", + f"focus={getattr(app.focused, 'id', None)}") + await c.press("escape") + await c.wait(lambda: not isinstance(app.screen, LoadOptionsScreen), 10) + @scenario("command_palette") async def s_command_palette(c: Ctx): |