fix(ghidra): reject address names owned by another symbol
Check function and label ownership in the existing namespace before renaming. Keep same-address renames idempotent and reject duplicate batch members inside their individual transactions. Extend real-provider namespace checks for single and batch collisions without conflating function and label fixture names. Validation: real Ghidra 12.1.4 host-native verification passed on macOS ARM64 with JDK 21.0.12.1, including public CLI/stdIO MCP, qualified-name round trips, rollback, source-byte immutability, and complete cleanup. Windows mutation authority remains unchanged. Fixes #1649
This commit is contained in:
parent
095fad9406
commit
aa7c5b7b5e
|
|
@ -80,12 +80,14 @@ import ghidra.program.database.mem.FileBytes;
|
|||
import ghidra.program.model.address.SegmentedAddress;
|
||||
import ghidra.program.model.reloc.Relocation;
|
||||
import java.io.ByteArrayOutputStream;
|
||||
import ghidra.program.model.symbol.Namespace;
|
||||
import ghidra.program.model.symbol.RefType;
|
||||
import ghidra.program.model.symbol.Reference;
|
||||
import ghidra.program.model.symbol.ReferenceIterator;
|
||||
import ghidra.program.model.symbol.SourceType;
|
||||
import ghidra.program.model.symbol.Symbol;
|
||||
import ghidra.program.model.symbol.SymbolIterator;
|
||||
import ghidra.program.model.symbol.SymbolTable;
|
||||
import ghidra.program.model.pcode.FunctionPrototype;
|
||||
import ghidra.program.model.pcode.HighFunction;
|
||||
import ghidra.program.model.pcode.HighParam;
|
||||
|
|
@ -1434,18 +1436,22 @@ public final class ReaGhidraBridge extends HeadlessScript {
|
|||
boolean commit = false;
|
||||
try {
|
||||
Function function = currentProgram.getFunctionManager().getFunctionAt(address);
|
||||
String expected = name;
|
||||
SymbolTable table = currentProgram.getSymbolTable();
|
||||
Symbol primary = function == null ? table.getPrimarySymbol(address) : function.getSymbol();
|
||||
Namespace namespace = primary == null
|
||||
? currentProgram.getGlobalNamespace()
|
||||
: primary.getParentNamespace();
|
||||
String expected = primary == null ? name : symbolLeafName(primary, name);
|
||||
for (Symbol symbol : table.getLabelOrFunctionSymbols(expected, namespace)) {
|
||||
if (!address.equals(symbol.getAddress())) return false;
|
||||
}
|
||||
if (function != null) {
|
||||
expected = symbolLeafName(function.getSymbol(), name);
|
||||
function.setName(expected, SourceType.USER_DEFINED);
|
||||
} else {
|
||||
var table = currentProgram.getSymbolTable();
|
||||
Symbol primary = table.getPrimarySymbol(address);
|
||||
if (primary != null && !primary.isDynamic()) {
|
||||
expected = symbolLeafName(primary, name);
|
||||
primary.setName(expected, SourceType.USER_DEFINED);
|
||||
} else {
|
||||
Symbol created = table.createLabel(address, name, SourceType.USER_DEFINED);
|
||||
Symbol created = table.createLabel(address, expected, SourceType.USER_DEFINED);
|
||||
created.setPrimary();
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ export async function verifyGhidraNamespaceAnnotations({
|
|||
assert.ok(symbol, `Missing demangled fixture global ${namespace}::value`);
|
||||
return { namespace, address: symbol.address };
|
||||
});
|
||||
await verifyDuplicateAddressNames(call, procedures, names);
|
||||
const ambiguous = await reject("procedure_address", { procedure: "same" });
|
||||
assert.equal(ambiguous.code, "invalid_request");
|
||||
for (const { address } of entries)
|
||||
|
|
@ -133,7 +134,8 @@ export async function verifyGhidraNamespaceAnnotations({
|
|||
});
|
||||
assert.deepEqual(unchanged.annotations, qualified.annotations);
|
||||
}
|
||||
await verifyAddressNames(call, [entries[0], entries[2], ...globals]);
|
||||
await verifyAddressNames(call, [entries[0], entries[2]], "function");
|
||||
await verifyAddressNames(call, globals, "label");
|
||||
for (const { namespace, address } of entries) {
|
||||
assert.equal(
|
||||
await call("set_address_name", {
|
||||
|
|
@ -177,24 +179,107 @@ export async function verifyGhidraNamespaceAnnotations({
|
|||
}
|
||||
}
|
||||
|
||||
async function verifyAddressNames(call, symbols) {
|
||||
for (const { namespace, address } of symbols) {
|
||||
async function verifyDuplicateAddressNames(call, procedures, names) {
|
||||
const functionEntries = [
|
||||
"rea_duplicate_function_a",
|
||||
"rea_duplicate_function_b",
|
||||
].map((name) => {
|
||||
const procedure = procedures.find(
|
||||
({ value }) => value === name || value === `_${name}`,
|
||||
);
|
||||
assert.ok(procedure, `Missing duplicate-name fixture function ${name}`);
|
||||
return { address: procedure.address, original: procedure.value };
|
||||
});
|
||||
const labelEntries = ["rea_duplicate_label_a", "rea_duplicate_label_b"].map(
|
||||
(name) => {
|
||||
const symbol = names.find(
|
||||
(item) =>
|
||||
(item.value === name || item.value === `_${name}`) &&
|
||||
item.symbol.primary,
|
||||
);
|
||||
assert.ok(symbol, `Missing duplicate-name fixture label ${name}`);
|
||||
return { address: symbol.address, original: symbol.value };
|
||||
},
|
||||
);
|
||||
|
||||
for (const [kind, entries] of [
|
||||
["function", functionEntries],
|
||||
["label", labelEntries],
|
||||
]) {
|
||||
const [owner, duplicate] = entries;
|
||||
const name = `rea_duplicate_${kind}`;
|
||||
assert.equal(
|
||||
await call("set_address_name", { address, name: "address_renamed" }),
|
||||
await call("set_address_name", { address: owner.address, name }),
|
||||
true,
|
||||
);
|
||||
const qualified = `${namespace}::address_qualified`;
|
||||
assert.equal(
|
||||
await call("set_address_name", { address: owner.address, name }),
|
||||
true,
|
||||
"Renaming the current owner to its existing name remains idempotent",
|
||||
);
|
||||
assert.equal(
|
||||
await call("set_address_name", { address: duplicate.address, name }),
|
||||
false,
|
||||
);
|
||||
assert.equal(
|
||||
await call("address_name", { address: duplicate.address }),
|
||||
duplicate.original,
|
||||
"A rejected duplicate leaves the other address unchanged",
|
||||
);
|
||||
|
||||
const batchName = `rea_batch_duplicate_${kind}`;
|
||||
assert.deepEqual(
|
||||
await call("set_addresses_names", {
|
||||
names: {
|
||||
[owner.address]: batchName,
|
||||
[duplicate.address]: batchName,
|
||||
},
|
||||
}),
|
||||
{ [owner.address]: true, [duplicate.address]: false },
|
||||
);
|
||||
assert.equal(
|
||||
await call("address_name", { address: owner.address }),
|
||||
batchName,
|
||||
);
|
||||
assert.equal(
|
||||
await call("address_name", { address: duplicate.address }),
|
||||
duplicate.original,
|
||||
"A failed batch member rolls back only its own address",
|
||||
);
|
||||
const owners = (await call("list_names")).filter(
|
||||
(item) => item.value === batchName && item.symbol.primary,
|
||||
);
|
||||
assert.deepEqual(
|
||||
owners.map(({ address }) => address),
|
||||
[owner.address],
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
async function verifyAddressNames(call, symbols, kind) {
|
||||
for (const { namespace, address } of symbols) {
|
||||
assert.equal(
|
||||
await call("set_address_name", {
|
||||
address,
|
||||
name: `address_renamed_${kind}`,
|
||||
}),
|
||||
true,
|
||||
);
|
||||
const qualified = `${namespace}::address_qualified_${kind}`;
|
||||
assert.equal(
|
||||
await call("set_address_name", { address, name: qualified }),
|
||||
true,
|
||||
);
|
||||
assert.equal(await call("address_name", { address }), qualified);
|
||||
assert.equal(
|
||||
await call("set_address_name", { address, name: "other::literal" }),
|
||||
await call("set_address_name", {
|
||||
address,
|
||||
name: `other::literal_${kind}`,
|
||||
}),
|
||||
true,
|
||||
);
|
||||
const literal = await call("address_name", { address });
|
||||
assert.equal(literal, `${namespace}::other::literal`);
|
||||
assert.equal(literal, `${namespace}::other::literal_${kind}`);
|
||||
assert.equal(
|
||||
await call("set_address_name", { address, name: literal }),
|
||||
true,
|
||||
|
|
@ -235,7 +320,7 @@ async function verifyAddressNames(call, symbols) {
|
|||
assert.deepEqual(
|
||||
await call("set_addresses_names", {
|
||||
names: {
|
||||
[valid.address]: `${valid.namespace}::batch_renamed`,
|
||||
[valid.address]: `${valid.namespace}::batch_renamed_${kind}`,
|
||||
[invalid.address]: `${invalid.namespace}::`,
|
||||
},
|
||||
}),
|
||||
|
|
@ -243,18 +328,18 @@ async function verifyAddressNames(call, symbols) {
|
|||
);
|
||||
assert.equal(
|
||||
await call("address_name", { address: valid.address }),
|
||||
`${valid.namespace}::batch_renamed`,
|
||||
`${valid.namespace}::batch_renamed_${kind}`,
|
||||
);
|
||||
assert.equal(
|
||||
await call("address_name", { address: invalid.address }),
|
||||
`${invalid.namespace}::address_qualified`,
|
||||
`${invalid.namespace}::address_qualified_${kind}`,
|
||||
);
|
||||
const names = await call("list_names");
|
||||
for (const { namespace, address } of symbols) {
|
||||
const expected =
|
||||
address === valid.address
|
||||
? `${namespace}::batch_renamed`
|
||||
: `${namespace}::address_qualified`;
|
||||
? `${namespace}::batch_renamed_${kind}`
|
||||
: `${namespace}::address_qualified_${kind}`;
|
||||
assert.ok(
|
||||
names.some(
|
||||
(item) =>
|
||||
|
|
|
|||
|
|
@ -10,6 +10,17 @@ volatile int rea_namespace_alpha_value
|
|||
volatile int rea_namespace_nested_value
|
||||
__asm__(REA_CPP_SYMBOL("_ZN5outer5inner5valueE")) = 1;
|
||||
|
||||
volatile int rea_duplicate_label_a = 3;
|
||||
volatile int rea_duplicate_label_b = 4;
|
||||
|
||||
__attribute__((noinline, used)) int rea_duplicate_function_a(int value) {
|
||||
return value + 1;
|
||||
}
|
||||
|
||||
__attribute__((noinline, used)) int rea_duplicate_function_b(int value) {
|
||||
return value + 2;
|
||||
}
|
||||
|
||||
__attribute__((noinline, used)) int rea_namespace_alpha(int value)
|
||||
__asm__(REA_CPP_SYMBOL("_ZN5alpha4sameEi"));
|
||||
__attribute__((noinline, used)) int rea_namespace_beta(int value)
|
||||
|
|
|
|||
Loading…
Reference in New Issue