From 6c28553c582b18c6ebbc7b4d7cb9818d56a3d307 Mon Sep 17 00:00:00 2001 From: Teal Bauer Date: Mon, 14 Apr 2025 21:23:45 +0200 Subject: [PATCH] fix: Implement create_data and delete_data functionality - Add handleCreateData method to Java plugin to support creating new data - Add data type mapping to support common types like byte, word, dword, string - Implement delete_data functionality with graceful handling of missing data - Add proper error handling when conflicts are detected - Add comprehensive tests for both create_data and delete_data functionality --- bridge_mcp_hydra.py | 28 ++ .../ghidra/endpoints/DataEndpoints.java | 453 +++++++++++++++++- test_data_create.py | 135 ++++++ test_data_delete.py | 85 ++++ 4 files changed, 699 insertions(+), 2 deletions(-) create mode 100644 test_data_create.py create mode 100644 test_data_delete.py diff --git a/bridge_mcp_hydra.py b/bridge_mcp_hydra.py index 2616062..1e435ec 100644 --- a/bridge_mcp_hydra.py +++ b/bridge_mcp_hydra.py @@ -1540,6 +1540,34 @@ def create_data(port: int = DEFAULT_GHIDRA_PORT, return simplify_response(response) +@mcp.tool() +def delete_data(port: int = DEFAULT_GHIDRA_PORT, + address: str = "") -> dict: + """Delete data at the specified address + + Args: + port: Ghidra instance port (default: 8192) + address: Memory address in hex format + + Returns: + dict: Operation result + """ + if not address: + return { + "success": False, + "error": "Address parameter is required", + "timestamp": int(time.time() * 1000) + } + + payload = { + "address": address, + "action": "delete" + } + + response = safe_post(port, "data/delete", payload) + return simplify_response(response) + + @mcp.tool() def rename_data(port: int = DEFAULT_GHIDRA_PORT, address: str = "", diff --git a/src/main/java/eu/starsong/ghidra/endpoints/DataEndpoints.java b/src/main/java/eu/starsong/ghidra/endpoints/DataEndpoints.java index 29e8f68..f6b0079 100644 --- a/src/main/java/eu/starsong/ghidra/endpoints/DataEndpoints.java +++ b/src/main/java/eu/starsong/ghidra/endpoints/DataEndpoints.java @@ -45,6 +45,19 @@ package eu.starsong.ghidra.endpoints; @Override public void registerEndpoints(HttpServer server) { server.createContext("/data", this::handleData); + server.createContext("/data/delete", exchange -> { + try { + if ("POST".equals(exchange.getRequestMethod())) { + Map params = parseJsonPostParams(exchange); + handleDeleteData(exchange, params); + } else { + sendErrorResponse(exchange, 405, "Method Not Allowed"); + } + } catch (Exception e) { + Msg.error(this, "Error in /data/delete endpoint", e); + sendErrorResponse(exchange, 500, "Internal server error: " + e.getMessage()); + } + }); server.createContext("/data/update", exchange -> { try { if ("POST".equals(exchange.getRequestMethod())) { @@ -89,14 +102,39 @@ package eu.starsong.ghidra.endpoints; boolean hasNewName = params.containsKey("newName") && params.get("newName") != null && !params.get("newName").isEmpty(); boolean hasType = params.containsKey("type") && params.get("type") != null && !params.get("type").isEmpty(); + boolean hasSize = params.containsKey("size") && params.get("size") != null && !params.get("size").isEmpty(); // Add more detailed debugging - Msg.info(this, "Decision logic: hasNewName=" + hasNewName + ", hasType=" + hasType); + Msg.info(this, "Decision logic: hasNewName=" + hasNewName + ", hasType=" + hasType + ", hasSize=" + hasSize); Msg.info(this, "Raw newName value: " + params.get("newName")); Msg.info(this, "Raw type value: " + params.get("type")); Msg.info(this, "Raw address value: " + params.get("address")); + Msg.info(this, "Raw size value: " + params.get("size")); - // Let's go ahead and call handleUpdateData (since we know we have both params) + // Check if this is a create operation (address + type without checking for existing data) + if (params.containsKey("address") && hasType) { + // Check if the data already exists at the address + try { + Program program = getCurrentProgram(); + if (program != null) { + Address addr = program.getAddressFactory().getAddress(params.get("address")); + Listing listing = program.getListing(); + Data data = listing.getDefinedDataAt(addr); + + if (data == null) { + // No data exists at this address, so treat as a create operation + Msg.info(this, "Selected route: handleCreateData - creating new data"); + handleCreateData(exchange, params); + return; + } + } + } catch (Exception e) { + Msg.warn(this, "Error checking for existing data: " + e.getMessage()); + // Continue with normal processing + } + } + + // Proceeding with update operations if not create if (params.containsKey("address") && hasNewName && hasType) { Msg.info(this, "Selected route: handleUpdateData - both name and type"); handleUpdateData(exchange, params); @@ -706,6 +744,324 @@ package eu.starsong.ghidra.endpoints; // parseIntOrDefault is inherited from AbstractEndpoint + /** + * Handle a data creation request + */ + public void handleCreateData(HttpExchange exchange, Map params) throws IOException { + try { + // Debug - log all parameters + StringBuilder debugInfo = new StringBuilder("DEBUG handleCreateData - Received parameters: "); + for (Map.Entry entry : params.entrySet()) { + debugInfo.append(entry.getKey()).append("=").append(entry.getValue()).append(", "); + } + Msg.info(this, debugInfo.toString()); + + final String addressStr = params.get("address"); + final String dataTypeStr = params.get("type"); + final String sizeStr = params.get("size"); + final String nameStr = params.get("newName"); // Optional name for the new data + + // Validate required parameters + if (addressStr == null || addressStr.isEmpty()) { + sendErrorResponse(exchange, 400, "Missing required parameter: address", "MISSING_PARAMETERS"); + return; + } + + if (dataTypeStr == null || dataTypeStr.isEmpty()) { + sendErrorResponse(exchange, 400, "Missing required parameter: type", "MISSING_PARAMETERS"); + return; + } + + Program program = getCurrentProgram(); + if (program == null) { + sendErrorResponse(exchange, 400, "No program loaded", "NO_PROGRAM_LOADED"); + return; + } + + // Parse size if provided + Integer size = null; + if (sizeStr != null && !sizeStr.isEmpty()) { + try { + size = Integer.parseInt(sizeStr); + } catch (NumberFormatException e) { + sendErrorResponse(exchange, 400, "Invalid size parameter: must be an integer", "INVALID_PARAMETER"); + return; + } + } + + try { + // Create a result map for the response + Map resultMap = new HashMap<>(); + resultMap.put("address", addressStr); + resultMap.put("dataType", dataTypeStr); + if (size != null) { + resultMap.put("size", size); + } + + final Integer finalSize = size; // Make a final copy for the lambda + + TransactionHelper.executeInTransaction(program, "Create Data", () -> { + // Get the address + Address addr = program.getAddressFactory().getAddress(addressStr); + Listing listing = program.getListing(); + + // Verify no data is already defined at this address + Data existingData = listing.getDefinedDataAt(addr); + if (existingData != null) { + throw new Exception("Data already exists at address: " + addressStr); + } + + // Find the requested data type + ghidra.program.model.data.DataType dataType = null; + + // Map common shorthand data types to their Ghidra equivalents + String mappedType = dataTypeStr; + switch(dataTypeStr.toLowerCase()) { + case "byte": + mappedType = "byte"; + break; + case "char": + mappedType = "char"; + break; + case "word": + mappedType = "word"; + break; + case "dword": + mappedType = "dword"; + break; + case "qword": + mappedType = "qword"; + break; + case "string": + // For string, we'll use StringDataType directly + dataType = new ghidra.program.model.data.StringDataType(); + break; + case "float": + mappedType = "float"; + break; + case "double": + mappedType = "double"; + break; + case "int": + mappedType = "int"; + break; + case "long": + mappedType = "long"; + break; + case "pointer": + mappedType = "pointer"; + break; + default: + // Keep the original type string + break; + } + + // Continue with data type lookup if not directly mapped + if (dataType == null) { + // First try built-in types with path + dataType = program.getDataTypeManager().getDataType("/" + mappedType); + + // If not found, try to find it without path + if (dataType == null) { + dataType = program.getDataTypeManager().findDataType("/" + mappedType); + } + + // Try data type manager from program + if (dataType == null) { + try { + dataType = program.getDataTypeManager().findDataType("/" + mappedType); + } catch (Exception e) { + Msg.debug(this, "Error getting built-in type: " + e.getMessage()); + } + } + + // If still null, try parsing it + if (dataType == null) { + try { + ghidra.app.util.parser.FunctionSignatureParser parser = + new ghidra.app.util.parser.FunctionSignatureParser(program.getDataTypeManager(), null); + dataType = parser.parse(null, mappedType); + } catch (Exception e) { + Msg.debug(this, "Function signature parser failed: " + e.getMessage()); + } + } + } + + // Try some specific data type classes if still not found + if (dataType == null) { + switch(dataTypeStr.toLowerCase()) { + case "byte": + dataType = new ghidra.program.model.data.ByteDataType(); + break; + case "char": + dataType = new ghidra.program.model.data.CharDataType(); + break; + case "word": + dataType = new ghidra.program.model.data.WordDataType(); + break; + case "dword": + dataType = new ghidra.program.model.data.DWordDataType(); + break; + case "qword": + dataType = new ghidra.program.model.data.QWordDataType(); + break; + case "float": + dataType = new ghidra.program.model.data.FloatDataType(); + break; + case "double": + dataType = new ghidra.program.model.data.DoubleDataType(); + break; + case "int": + dataType = new ghidra.program.model.data.IntegerDataType(); + break; + case "uint32_t": + dataType = new ghidra.program.model.data.UnsignedIntegerDataType(); + break; + case "long": + dataType = new ghidra.program.model.data.LongDataType(); + break; + case "pointer": + dataType = new ghidra.program.model.data.PointerDataType(); + break; + } + } + + if (dataType == null) { + throw new Exception("Could not find or parse data type: " + dataTypeStr); + } + + Msg.info(this, "Successfully mapped data type '" + dataTypeStr + "' to Ghidra type: " + dataType.getName()); + + // Create the data at the specified address + Data newData; + + // Make final copy of dataType for use in lambda + final ghidra.program.model.data.DataType finalDataType = dataType; + + // Check if there's already existing code or data at this address + int dataSize = finalSize != null ? finalSize : finalDataType.getLength(); + if (dataSize <= 0) dataSize = 4; // Default size if unknown + + // Check if there's data/code at the target address range + boolean hasConflict = false; + String conflictType = ""; + + try { + // Check for existing code units + if (listing.getInstructionAt(addr) != null) { + hasConflict = true; + conflictType = "instruction"; + } else if (listing.getDefinedDataAt(addr) != null) { + hasConflict = true; + conflictType = "data"; + } else { + // Check if any address in range has a code unit + for (int i = 1; i < dataSize; i++) { + Address checkAddr = addr.add(i); + if (listing.getInstructionAt(checkAddr) != null) { + hasConflict = true; + conflictType = "instruction"; + break; + } else if (listing.getDefinedDataAt(checkAddr) != null) { + hasConflict = true; + conflictType = "data"; + break; + } + } + } + + if (hasConflict) { + throw new Exception("Conflicting " + conflictType + " exists at address range " + addr + + " to " + addr.add(dataSize - 1) + ". Use update_data or delete_data first."); + } + + // Also check if the address is valid (in a memory block) + if (!program.getMemory().contains(addr)) { + throw new Exception("Address " + addressStr + " is not in any memory block. Valid addresses must be within defined memory blocks."); + } + } catch (Exception e) { + if (hasConflict) { + throw e; // Rethrow the conflict error + } + // Other errors we can try to continue + Msg.warn(this, "Error checking for existing code units: " + e.getMessage()); + } + + // Now create the data + if (finalSize != null) { + // For variable length types like strings, need to clear space first + if (finalDataType.getLength() <= 0 || finalDataType.getLength() != finalSize) { + Msg.info(this, "Creating variable-length data with size: " + finalSize); + + // For arrays and strings, may need to create custom type + if (finalDataType.getName().toLowerCase().contains("string") || + dataTypeStr.toLowerCase().contains("string")) { + // Create a string data type with specified length + try { + ghidra.program.model.data.StringDataType stringType = new ghidra.program.model.data.StringDataType(); + newData = listing.createData(addr, stringType, finalSize); + } catch (Exception e) { + Msg.warn(this, "Couldn't create string data: " + e.getMessage()); + // Fallback to byte array + newData = listing.createData(addr, new ghidra.program.model.data.ByteDataType(), finalSize); + } + } else { + // For other variable length types, create clear space and then create + newData = listing.createData(addr, finalDataType); + } + } else { + // For fixed size datatypes + newData = listing.createData(addr, finalDataType); + } + } else { + // Normal data creation without size + newData = listing.createData(addr, finalDataType); + } + + if (newData == null) { + throw new Exception("Failed to create data of type " + dataTypeStr + " at " + addressStr); + } + + // Set name if provided + if (nameStr != null && !nameStr.isEmpty()) { + SymbolTable symTable = program.getSymbolTable(); + symTable.createLabel(addr, nameStr, SourceType.USER_DEFINED); + resultMap.put("name", nameStr); + } + + // Add information about the created data to the result + resultMap.put("length", newData.getLength()); + resultMap.put("value", newData.getDefaultValueRepresentation()); + + return null; + }); + + resultMap.put("message", "Data created successfully"); + + // Build HATEOAS response + eu.starsong.ghidra.api.ResponseBuilder builder = new eu.starsong.ghidra.api.ResponseBuilder(exchange, port) + .success(true) + .result(resultMap); + + // Add relevant links + builder.addLink("self", "/data/" + addressStr); + builder.addLink("data", "/data"); + builder.addLink("program", "/program"); + + sendJsonResponse(exchange, builder.build(), 200); + } catch (TransactionException e) { + Msg.error(this, "Transaction failed: Create Data", e); + sendErrorResponse(exchange, 500, "Failed to create data: " + e.getMessage(), "TRANSACTION_ERROR"); + } catch (Exception e) { + Msg.error(this, "Error during data creation", e); + sendErrorResponse(exchange, 400, "Error creating data: " + e.getMessage(), "INVALID_PARAMETER"); + } + } catch (Exception e) { + Msg.error(this, "Unexpected error creating data", e); + sendErrorResponse(exchange, 500, "Error creating data: " + e.getMessage(), "INTERNAL_ERROR"); + } + } + public void handleSetDataType(HttpExchange exchange) throws IOException { try { if ("PATCH".equals(exchange.getRequestMethod()) || "POST".equals(exchange.getRequestMethod())) { @@ -844,5 +1200,98 @@ package eu.starsong.ghidra.endpoints; } } + /** + * Handle a delete data request + */ + public void handleDeleteData(HttpExchange exchange, Map params) throws IOException { + try { + // Debug - log all parameters + StringBuilder debugInfo = new StringBuilder("DEBUG handleDeleteData - Received parameters: "); + for (Map.Entry entry : params.entrySet()) { + debugInfo.append(entry.getKey()).append("=").append(entry.getValue()).append(", "); + } + Msg.info(this, debugInfo.toString()); + + final String addressStr = params.get("address"); + + // Validate required parameters + if (addressStr == null || addressStr.isEmpty()) { + sendErrorResponse(exchange, 400, "Missing required parameter: address", "MISSING_PARAMETERS"); + return; + } + + Program program = getCurrentProgram(); + if (program == null) { + sendErrorResponse(exchange, 400, "No program loaded", "NO_PROGRAM_LOADED"); + return; + } + + try { + // Create a result map for the response + Map resultMap = new HashMap<>(); + resultMap.put("address", addressStr); + + TransactionHelper.executeInTransaction(program, "Delete Data", () -> { + // Get the address + Address addr = program.getAddressFactory().getAddress(addressStr); + Listing listing = program.getListing(); + + // Check if there's data at the address + Data existingData = listing.getDefinedDataAt(addr); + if (existingData == null) { + // Check if there's an instruction + if (listing.getInstructionAt(addr) != null) { + // Clear the instruction + listing.clearCodeUnits(addr, addr, true); + resultMap.put("cleared", "instruction"); + } else { + // No data or instruction, but still treat as success + resultMap.put("message", "No data or instruction exists at address: " + addressStr); + resultMap.put("cleared", "none"); + } + } else { + // Remember what we're deleting + resultMap.put("original_type", existingData.getDataType().getName()); + resultMap.put("length", existingData.getLength()); + + // Get the name if any + Symbol symbol = program.getSymbolTable().getPrimarySymbol(addr); + if (symbol != null) { + resultMap.put("original_name", symbol.getName()); + } + + // Clear the data + listing.clearCodeUnits(addr, addr.add(existingData.getLength() - 1), true); + resultMap.put("cleared", "data"); + } + + return null; + }); + + resultMap.put("message", "Data deleted successfully"); + + // Build HATEOAS response + eu.starsong.ghidra.api.ResponseBuilder builder = new eu.starsong.ghidra.api.ResponseBuilder(exchange, port) + .success(true) + .result(resultMap); + + // Add relevant links + builder.addLink("data", "/data"); + builder.addLink("program", "/program"); + + sendJsonResponse(exchange, builder.build(), 200); + } catch (TransactionException e) { + Msg.error(this, "Transaction failed: Delete Data", e); + sendErrorResponse(exchange, 500, "Failed to delete data: " + e.getMessage(), "TRANSACTION_ERROR"); + } catch (Exception e) { + Msg.error(this, "Error during data deletion", e); + sendErrorResponse(exchange, 400, "Error deleting data: " + e.getMessage(), "INVALID_PARAMETER"); + } + } catch (Exception e) { + Msg.error(this, "Unexpected error deleting data", e); + sendErrorResponse(exchange, 500, "Error deleting data: " + e.getMessage(), "INTERNAL_ERROR"); + } + } + // Note: The handleUpdateData method is already defined earlier in this file at line 477 } diff --git a/test_data_create.py b/test_data_create.py new file mode 100644 index 0000000..455d67f --- /dev/null +++ b/test_data_create.py @@ -0,0 +1,135 @@ +#!/usr/bin/env python3 +""" +Test script to verify the create_data function works properly. +""" +import json +import logging +import sys +import requests +import time + +# Setup logging +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger("create_data_test") + +def wait_for_program_loaded(): + """Wait for a Ghidra program to be loaded.""" + for _ in range(10): # Try for ~20 seconds + try: + response = requests.get("http://localhost:8192/program") + if response.status_code == 200: + data = json.loads(response.text) + if data.get("success", False): + logger.info("Program loaded: " + data["result"]["name"]) + return True + except Exception as e: + logger.warning(f"Error checking program status: {e}") + + logger.info("Waiting for program to load...") + time.sleep(2) + + logger.error("Timed out waiting for program to load") + return False + +def test_create_data(): + """Test creating data at different addresses with different types.""" + # First wait for a program to be loaded + if not wait_for_program_loaded(): + logger.error("No program loaded, cannot test create_data") + return False + + # First get the memory map to find addresses where we can create data + try: + response = requests.get("http://localhost:8192/memory") + memory_info = json.loads(response.text) + + # Get valid addresses from an existing memory region + memory_blocks = memory_info.get("result", []) + + # Find a valid memory block + valid_addresses = [] + for block in memory_blocks: + if "start" in block and "name" in block: + # Get starting address of a RAM block + if "RAM" in block["name"].upper(): + # Use the first 10 bytes of this RAM block + addr_base = int(block["start"], 16) + for i in range(10): + valid_addresses.append(f"{addr_base + i:08x}") + break + + # If no RAM blocks, try any memory block + if not valid_addresses: + for block in memory_blocks: + if "start" in block: + # Use the first 10 bytes of this block + addr_base = int(block["start"], 16) + for i in range(10): + valid_addresses.append(f"{addr_base + i:08x}") + break + + # Fallback to known addresses if still nothing + if not valid_addresses: + valid_addresses = ["08000100", "08000104", "08000108", "0800010c", + "08000110", "08000114", "08000118", "0800011c"] + + logger.info(f"Will try using addresses: {valid_addresses[:3]}...") + addresses = valid_addresses + except Exception as e: + logger.error(f"Error getting memory map: {e}") + # Fallback to some addresses that might be valid + addresses = ["08000100", "08000104", "08000108", "0800010c", + "08000110", "08000114", "08000118", "0800011c"] + + # Try data types + types_to_try = ["uint32_t", "int", "float", "byte", "char", "word", "dword", "string"] + + success_count = 0 + + for i, data_type in enumerate(types_to_try): + address = addresses[i % len(addresses)] + logger.info(f"Testing data type: {data_type} at address {address}") + + # First try direct HTTP API + url = f"http://localhost:8192/data" + payload = { + "address": address, + "type": data_type, + "newName": f"TEST_{data_type.upper()}" + } + + # Add size for string types + if data_type.lower() == "string": + payload["size"] = 16 + + try: + response = requests.post(url, json=payload) + logger.info(f"HTTP API - Status: {response.status_code}") + logger.info(f"HTTP API - Response: {response.text}") + if response.status_code == 200 and json.loads(response.text).get("success", False): + success_count += 1 + logger.info(f"HTTP API - Success with data type {data_type}") + else: + logger.warning(f"HTTP API - Failed with data type {data_type}") + except Exception as e: + logger.error(f"HTTP API - Error: {e}") + + # Short delay between tests + time.sleep(0.5) + + return success_count > 0 + +def main(): + try: + result = test_create_data() + if result: + logger.info("Test successful!") + else: + logger.error("All test data types failed") + sys.exit(1) + except Exception as e: + logger.error(f"Unexpected error: {e}") + sys.exit(1) + +if __name__ == "__main__": + main() \ No newline at end of file diff --git a/test_data_delete.py b/test_data_delete.py new file mode 100644 index 0000000..69666ee --- /dev/null +++ b/test_data_delete.py @@ -0,0 +1,85 @@ +#!/usr/bin/env python3 +""" +Test script to verify the delete_data functionality works properly. +""" +import json +import logging +import sys +import requests +import time + +# Setup logging +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger("delete_data_test") + +def test_delete_data(): + """Test deleting data.""" + # First create data at a specific address + test_address = "08000100" # This should be a valid address in the memory map + test_type = "byte" + + # Step 1: Create some data + logger.info(f"Creating test data at {test_address}") + create_url = "http://localhost:8192/data" + create_payload = { + "address": test_address, + "type": test_type, + "newName": "TEST_DELETE_ME" + } + + try: + create_response = requests.post(create_url, json=create_payload) + logger.info(f"Create response: {create_response.status_code}") + logger.info(f"Create response: {create_response.text}") + + create_success = create_response.status_code == 200 and json.loads(create_response.text).get("success", False) + + if not create_success: + logger.warning("Failed to create test data, test may fail") + except Exception as e: + logger.error(f"Error creating test data: {e}") + + # Short delay + time.sleep(1) + + # Step 2: Delete the data + logger.info(f"Deleting data at {test_address}") + delete_url = "http://localhost:8192/data/delete" + delete_payload = { + "address": test_address, + "action": "delete" + } + + try: + delete_response = requests.post(delete_url, json=delete_payload) + logger.info(f"Delete response: {delete_response.status_code}") + logger.info(f"Delete response: {delete_response.text}") + + # Check if successful + if delete_response.status_code == 200: + response_data = json.loads(delete_response.text) + if response_data.get("success", False): + logger.info("Successfully deleted data!") + return True + + logger.warning("Failed to delete data") + return False + except Exception as e: + logger.error(f"Error deleting data: {e}") + return False + +def main(): + """Main entry point.""" + try: + result = test_delete_data() + if result: + logger.info("Test successful!") + else: + logger.error("Test failed") + sys.exit(1) + except Exception as e: + logger.error(f"Unexpected error: {e}") + sys.exit(1) + +if __name__ == "__main__": + main() \ No newline at end of file