GP-4097: last change

GP-4097: last change
GP-4097: per review comments
GP-4079: once more w/overlays
GP-4097: working again, post-review
GP-4097: broken for the moment
GP-4097: post-review changes
GP-4097: added processor-defined label options
GP-4097: consolidated addressSpace handling w/ fixes non-RAM

Ghidra/Features/Sarif/src/main/java/sarif/DEVNOTES.txt

@@ -4,10 +4,13 @@ Random notes:
    including functions and libraries and post- to guarantee namespaces have already been created
    for locals
    
+2. Overlay spaces are currently flagged using artifactLocation::uri on Location; address spaces with different names
+   are flagged in "fullyQualifiedName".
+   
+3: "Location is used to convey addresses; "location" is used generally for non-address information, e.g. file::offset.
+
 Code differences from export/re-import:
 
 1. There may be missing parameter datatypes for parameters in a FunctionDefinition used inside a structure or union.
 
 2. Datatypes like "RTTIBaseDescriptor *32 _((image-base-relative)) *32 _((image-base-relative))" are not handled correctly.
-
-3. Modified/multiple ProgramTrees will not be imported correctly.  Appears to be no way to fix this with the current API.

Ghidra/Features/Sarif/src/main/java/sarif/SarifController.java

@@ -24,7 +24,6 @@ import java.util.Map;
 import java.util.Set;
 
 import com.contrastsecurity.sarif.Location;
-import com.contrastsecurity.sarif.LogicalLocation;
 import com.contrastsecurity.sarif.Result;
 import com.contrastsecurity.sarif.SarifSchema210;
 
@@ -35,7 +34,6 @@ import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSetView;
 import ghidra.program.model.listing.BookmarkManager;
 import ghidra.program.model.listing.CodeUnit;
-import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.Program;
 import ghidra.service.graph.AttributedGraph;
 import ghidra.service.graph.EmptyGraphType;
@@ -200,13 +198,6 @@ public class SarifController implements ObjectSelectedListener<Map<String, Objec
 		coloringService.setBackgroundColor(addr, addr, color);
 	}
 
-	public Address longToAddress(Object lval) {
-		if (lval instanceof Long) {
-			return getProgram().getAddressFactory().getDefaultAddressSpace().getAddress((Long) lval);
-		}
-		return getProgram().getAddressFactory().getDefaultAddressSpace().getAddress((Integer) lval);
-	}
-
 	/**
 	 * Get listing addresses associated with a result
 	 *
@@ -227,28 +218,8 @@ public class SarifController implements ObjectSelectedListener<Map<String, Objec
 		return addrs;
 	}
 
-	public Address locationToAddress(Location loc) {
+	public Address locationToAddress(Location location) {
-		if (loc.getPhysicalLocation() != null) {
+		return SarifUtils.locationToAddress(location, program);
-			return longToAddress(loc.getPhysicalLocation().getAddress().getAbsoluteAddress());
-		}
-		if (loc.getLogicalLocations() != null) {
-			Set<LogicalLocation> logicalLocations = loc.getLogicalLocations();
-			for (LogicalLocation logLoc : logicalLocations) {
-				switch (logLoc.getKind()) {
-				case "function":
-					String fname = logLoc.getName();
-					for (Function func : getProgram().getFunctionManager().getFunctions(true)) {
-						if (fname.equals(func.getName())) {
-							return func.getEntryPoint();
-						}
-					}
-					break;
-				default:
-					Msg.error(this, "Unknown logical location to handle: " + logLoc.toString());
-				}
-			}
-		}
-		return null;
 	}
 
 	@SuppressWarnings("unchecked")

Ghidra/Features/Sarif/src/main/java/sarif/SarifProgramOptions.java

@@ -20,6 +20,7 @@ import java.util.List;
 
 import ghidra.app.util.Option;
 import ghidra.app.util.OptionException;
+import ghidra.app.util.opinion.AbstractProgramLoader;
 
 /**
  * A class to hold SARIF options.
@@ -94,6 +95,8 @@ public class SarifProgramOptions {
 	private boolean relocationTable = true;
 	private boolean entryPoints = true;
 	private boolean externalLibraries = true;
+	private boolean applyProcDefinedLabels = false;
+	private boolean anchorProcDefinedLabels = true;
 
 	/**
 	 * Returns an array of importer options representing
@@ -146,6 +149,8 @@ public class SarifProgramOptions {
 		optionList.add(new Option("Relocation Table", Boolean.valueOf(isRelocationTable())));
 		optionList.add(new Option("Entry Points", Boolean.valueOf(isEntryPoints())));
 		optionList.add(new Option("External Libraries", Boolean.valueOf(isExternalLibraries())));
+		optionList.add(new Option(AbstractProgramLoader.APPLY_LABELS_OPTION_NAME, Boolean.valueOf(applyProcDefinedLabels())));
+		optionList.add(new Option(AbstractProgramLoader.ANCHOR_LABELS_OPTION_NAME, Boolean.valueOf(anchorProcDefinedLabels())));
 
 		return optionList;
 	}
@@ -234,6 +239,12 @@ public class SarifProgramOptions {
 			else if (optName.equals("External Libraries")) {
 				setExternalLibraries(val);
 			}
+			else if (optName.equals("Apply Processor Defined Labels")) {
+				setExternalLibraries(val);
+			}
+			else if (optName.equals("Anchor Processor Defined Labels")) {
+				setExternalLibraries(val);
+			}
 			else {
 				throw new OptionException("Unknown option: " + optName);
 			}
@@ -604,6 +615,34 @@ public class SarifProgramOptions {
 		overwriteSymbolConflicts = b;
 	}
 
+	/**
+	 * @return true if the processor-defined labels should be applied
+	 */
+	public boolean applyProcDefinedLabels() {
+		return applyProcDefinedLabels;
+	}
+
+	/**
+	 * @param b true if the processor-defined labels should be applied
+	 */
+	public void setApplyProcDefinedLabels(boolean b) {
+		applyProcDefinedLabels = b;
+	}
+
+	/**
+	 * @param b trueif the processor-defined labels should be anchored
+	 */
+	public void setAnchorProcDefinedLabels(boolean b) {
+		anchorProcDefinedLabels = b;
+	}
+
+	/**
+	 * @return true true if the processor-defined labels should be anchored
+	 */
+	public boolean anchorProcDefinedLabels() {
+		return anchorProcDefinedLabels;
+	}
+
 	public void setAddToProgram(boolean addToProgram) {
 		this.addToProgram = addToProgram;
 	}

Ghidra/Features/Sarif/src/main/java/sarif/SarifUtils.java

@@ -16,21 +16,152 @@
 package sarif;
 
 import java.io.ByteArrayInputStream;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.List;
-import java.util.Set;
 
 import org.bouncycastle.util.encoders.Base64;
 
-import com.contrastsecurity.sarif.Artifact;
+import com.contrastsecurity.sarif.*;
-import com.contrastsecurity.sarif.ArtifactContent;
+import com.google.gson.JsonArray;
-import com.contrastsecurity.sarif.ReportingDescriptor;
+import com.google.gson.JsonObject;
-import com.contrastsecurity.sarif.ReportingDescriptorReference;
+
-import com.contrastsecurity.sarif.Run;
+import ghidra.framework.store.LockException;
-import com.contrastsecurity.sarif.ToolComponent;
+import ghidra.program.model.address.*;
+import ghidra.program.model.address.Address;
+import ghidra.program.model.listing.Program;
+import ghidra.util.InvalidNameException;
+import ghidra.util.exception.DuplicateNameException;
 
 public class SarifUtils {
 
+	// NB: We're abusing "fullyQualifiedName" and "artifactLocation/uri" here.
+	// For our purposes:
+	// fullyQualifiedName <= addressSpace name for overlays or non-TYPE_RAM
+	// addresses
+	// artifactLocation/uri <= the overlayED space name (typically OTHER)
+
+	public static JsonArray setLocations(Address min, Address max) {
+		AddressSet set = new AddressSet(min, max);
+		return setLocations(set);
+	}
+
+	public static JsonArray setLocations(AddressSetView set) {
+		JsonArray locations = new JsonArray();
+		AddressRangeIterator addressRanges = set.getAddressRanges();
+		while (addressRanges.hasNext()) {
+			JsonObject element = new JsonObject();
+			locations.add(element);
+			AddressRange next = addressRanges.next();
+			JsonObject ploc = new JsonObject();
+			element.add("physicalLocation", ploc);
+			JsonObject address = new JsonObject();
+			ploc.add("address", address);
+			address.addProperty("absoluteAddress", next.getMinAddress().getOffset());
+			address.addProperty("length", next.getLength());
+			Address minAddress = next.getMinAddress();
+			AddressSpace addressSpace = minAddress.getAddressSpace();
+			if (!addressSpace.showSpaceName()) {
+				continue;
+			}
+
+			// type != TYPE_RAM || isOverlaySpace()
+			address.addProperty("fullyQualifiedName", addressSpace.getName());
+			if (!(addressSpace instanceof OverlayAddressSpace ospace)) {
+				continue;
+			}
+
+			JsonObject artifact = new JsonObject();
+			ploc.add("artifactLocation", artifact);
+			String name = ospace.getOverlayedSpace().getName();
+			artifact.addProperty("uri", name);
+		}
+		return locations;
+	}
+
+	@SuppressWarnings("unchecked")
+	public static AddressSet getLocations(Map<String, Object> result, Program program,
+			AddressSet set) throws AddressOverflowException {
+		if (set == null) {
+			set = new AddressSet();
+		}
+		List<Location> locations = (List<Location>) result.get("Locations");
+		if (locations == null) {
+			return set;
+		}
+		for (Location location : locations) {
+			AddressRange range = locationToRange(location, program);
+			if (range != null) {
+				set.add(range);
+			}
+		}
+		return set;
+	}
+
+	public static AddressRange locationToRange(Location location, Program program)
+			throws AddressOverflowException {
+		PhysicalLocation physicalLocation = location.getPhysicalLocation();
+		long len = physicalLocation.getAddress().getLength();
+		Address addr = locationToAddress(location, program);
+		return addr == null ? null : new AddressRangeImpl(addr, len);
+	}
+
+	public static Address locationToAddress(Location location, Program program) {
+		if (location.getPhysicalLocation() != null) {
+			AddressFactory af = program.getAddressFactory();
+			AddressSpace base = af.getDefaultAddressSpace();
+
+			PhysicalLocation physicalLocation = location.getPhysicalLocation();
+			Long addr = physicalLocation.getAddress().getAbsoluteAddress();
+			String fqn = physicalLocation.getAddress().getFullyQualifiedName();
+			if (fqn == null) {
+				return longToAddress(base, addr);
+			}
+			if (fqn.equals("NO ADDRESS")) {
+				return null;
+			}
+
+			ArtifactLocation artifact = physicalLocation.getArtifactLocation();
+			if (artifact == null) { // Not an overlay
+				AddressSpace space = getAddressSpace(program, fqn, base);
+				return longToAddress(space, addr);
+			}
+
+			// Overlay
+			String uri = artifact.getUri();
+			base = program.getAddressFactory().getAddressSpace(uri);
+			if (base == null) {
+				try {
+					base = program.createOverlaySpace(fqn, base);
+				}
+				catch (IllegalStateException | DuplicateNameException | InvalidNameException
+						| LockException e) {
+					throw new RuntimeException("Attempt to create " + fqn + " failed!");
+				}
+			}
+			AddressSpace space = getAddressSpace(program, fqn, base);
+			return longToAddress(space, addr);
+		}
+		return null;
+	}
+
+	private static AddressSpace getAddressSpace(Program program, String fqn, AddressSpace base) {
+		AddressSpace space = program.getAddressFactory().getAddressSpace(fqn);
+		if (space != null) {
+			return space;
+		}
+		try {
+			space = program.createOverlaySpace(fqn, base);
+		}
+		catch (IllegalStateException | DuplicateNameException | InvalidNameException
+				| LockException e) {
+			throw new RuntimeException("Attempt to create " + fqn + " failed!");
+		}
+		return space;
+	}
+
+	public static Address longToAddress(AddressSpace space, Long addr) {
+		return space.getAddressInThisSpaceOnly(addr);
+	}
+
 	public static ByteArrayInputStream getArtifactContent(Artifact artifact) {
 		ArtifactContent content = artifact.getContents();
 		String b64 = content.getBinary();
@@ -38,17 +169,19 @@ public class SarifUtils {
 		return new ByteArrayInputStream(decoded);
 	}
 
-	public static ReportingDescriptor getTaxaValue(ReportingDescriptorReference taxa, ToolComponent taxonomy) {
+	public static ReportingDescriptor getTaxaValue(ReportingDescriptorReference taxa,
+			ToolComponent taxonomy) {
 		List<ReportingDescriptor> view = new ArrayList<>(taxonomy.getTaxa());
 		return view.get(taxa.getIndex().intValue());
 	}
 
-	public static ToolComponent getTaxonomy(ReportingDescriptorReference taxa, Set<ToolComponent> taxonomies) {
+	public static ToolComponent getTaxonomy(ReportingDescriptorReference taxa,
+			Set<ToolComponent> taxonomies) {
 		Object idx = taxa.getToolComponent().getIndex();
 		if (idx == null) {
 			List<ToolComponent> view = new ArrayList<>(taxonomies);
-			idx= taxa.getIndex();
+			idx = taxa.getIndex();
-			return view.get(idx instanceof Long ? ((Long)idx).intValue() : (Integer) idx);
+			return view.get(idx instanceof Long ? ((Long) idx).intValue() : (Integer) idx);
 		}
 		for (ToolComponent taxonomy : taxonomies) {
 			if (taxonomy.getName().equals(taxa.getToolComponent().getName())) {

Ghidra/Features/Sarif/src/main/java/sarif/export/SarifObject.java

@@ -20,11 +20,9 @@ import com.google.gson.JsonElement;
 import com.google.gson.JsonObject;
 
 import ghidra.program.model.address.Address;
-import ghidra.program.model.address.AddressRange;
-import ghidra.program.model.address.AddressRangeIterator;
 import ghidra.program.model.address.AddressSetView;
-import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.data.ISF.IsfObject;
+import sarif.SarifUtils;
 
 public class SarifObject implements IsfObject {
 
@@ -71,23 +69,8 @@ public class SarifObject implements IsfObject {
 
 	protected void writeLocations(Address min, Address max) {
 		if (SARIF) {
-			locations = new JsonArray();
+			locations = SarifUtils.setLocations(min, max);
-			JsonObject element = new JsonObject();
+		} else {
-			locations.add(element);
-			JsonObject ploc = new JsonObject();
-			element.add("physicalLocation", ploc);
-			JsonObject address = new JsonObject();
-			ploc.add("address", address);
-			address.addProperty("absoluteAddress", min.getOffset());
-			address.addProperty("length", max.subtract(min) + 1);
-			Address minAddress = min;
-			if (minAddress.getAddressSpace().getType() != AddressSpace.TYPE_RAM) {
-				JsonObject artifact = new JsonObject();
-				ploc.add("artifactLocation", artifact);
-				artifact.addProperty("uri", minAddress.toString());
-			}
-		}
-		else {
 			element.addProperty("startAddress", min.toString(true));
 			element.addProperty("stopAddress", max.toString(true));
 		}
@@ -95,27 +78,8 @@ public class SarifObject implements IsfObject {
 
 	protected void writeLocations(AddressSetView set) {
 		if (SARIF) {
-			locations = new JsonArray();
+			locations = SarifUtils.setLocations(set);
-			AddressRangeIterator addressRanges = set.getAddressRanges();
+		} else {
-			while (addressRanges.hasNext()) {
-				JsonObject element = new JsonObject();
-				locations.add(element);
-				AddressRange next = addressRanges.next();
-				JsonObject ploc = new JsonObject();
-				element.add("physicalLocation", ploc);
-				JsonObject address = new JsonObject();
-				ploc.add("address", address);
-				address.addProperty("absoluteAddress", next.getMinAddress().getOffset());
-				address.addProperty("length", next.getLength());
-				Address minAddress = next.getMinAddress();
-				if (minAddress.getAddressSpace().getType() != AddressSpace.TYPE_RAM) {
-					JsonObject artifact = new JsonObject();
-					ploc.add("artifactLocation", artifact);
-					artifact.addProperty("uri", minAddress.toString());
-				}
-			}
-		}
-		else {
 			element.addProperty("startAddress", set.getMinAddress().toString(true));
 			element.addProperty("stopAddress", set.getMaxAddress().toString(true));
 		}

Ghidra/Features/Sarif/src/main/java/sarif/export/mm/ExtMemoryMap.java

@@ -18,8 +18,6 @@ package sarif.export.mm;
 import java.io.IOException;
 
 import ghidra.program.model.address.AddressRange;
-import ghidra.program.model.address.AddressSpace;
-import ghidra.program.model.address.OverlayAddressSpace;
 import ghidra.program.model.data.ISF.IsfObject;
 import ghidra.program.model.mem.MemoryBlock;
 import ghidra.program.model.mem.MemoryBlockSourceInfo;
@@ -30,8 +28,6 @@ public class ExtMemoryMap implements IsfObject {
 
 	String name;
 	String kind;
-	String overlaySpace;
-	String overlayedSpace;
 	String comment;
 	boolean isVolatile;
 	String type;
@@ -52,12 +48,6 @@ public class ExtMemoryMap implements IsfObject {
 
 		name = block.getName();
 		kind = permissions;
-		AddressSpace space = range.getAddressSpace();
-		if (space instanceof OverlayAddressSpace) {
-			OverlayAddressSpace oSpace = (OverlayAddressSpace) space;
-			overlaySpace = oSpace.getName();
-			overlayedSpace = oSpace.getOverlayedSpace().getName();
-		}
 		if (block.getComment() != null) {
 			comment = block.getComment();
 		}

Ghidra/Features/Sarif/src/main/java/sarif/export/symbols/ExtSymbol.java

@@ -33,6 +33,7 @@ public class ExtSymbol implements IsfObject {
 	String type;
 	String sourceType;
 	boolean primary;
+	boolean pinned;
 
 	public ExtSymbol(Symbol symbol) {
 		name = symbol.getName();
@@ -40,6 +41,7 @@ public class ExtSymbol implements IsfObject {
 		kind = checkGlobal(symbol) ? "global" : "local";
 		sourceType = symbol.getSource().toString();
 		primary = symbol.isPrimary();
+		pinned = symbol.isPinned();
 		if (symbol instanceof ClassSymbol) {
 			type = "class";
 		} else if (symbol instanceof LibrarySymbol) {

Ghidra/Features/Sarif/src/main/java/sarif/managers/MemoryMapSarifMgr.java

@@ -30,7 +30,6 @@ import com.google.gson.JsonArray;
 import generic.stl.Pair;
 import ghidra.app.util.MemoryBlockUtils;
 import ghidra.app.util.importer.MessageLog;
-import ghidra.framework.store.LockException;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressFactory;
 import ghidra.program.model.address.AddressOverflowException;
@@ -39,17 +38,14 @@ import ghidra.program.model.address.AddressRangeImpl;
 import ghidra.program.model.address.AddressRangeIterator;
 import ghidra.program.model.address.AddressSet;
 import ghidra.program.model.address.AddressSetView;
-import ghidra.program.model.address.AddressSpace;
-import ghidra.program.model.address.OverlayAddressSpace;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.mem.Memory;
 import ghidra.program.model.mem.MemoryBlock;
-import ghidra.util.InvalidNameException;
 import ghidra.util.exception.CancelledException;
-import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.task.TaskLauncher;
 import ghidra.util.task.TaskMonitor;
 import sarif.SarifProgramOptions;
+import sarif.SarifUtils;
 import sarif.export.SarifWriterTask;
 import sarif.export.mm.SarifMemoryMapWriter;
 
@@ -87,9 +83,13 @@ public class MemoryMapSarifMgr extends SarifMgr {
 			Program program, TaskMonitor monitor) throws FileNotFoundException, AddressOverflowException {
 
 		String name = (String) result.get("name");
-		AddressSet set = getLocations(result, null);
+		AddressSet set = SarifUtils.getLocations(result, program, null);
-		Address addr = set.getMinAddress();
+		Address blockAddress = set.getMinAddress();
-		int length = (int) set.getMaxAddress().subtract(addr) + 1;
+		if (set.getNumAddressRanges() != 1) {
+			throw new RuntimeException(
+					"Unexpected number of ranges for block @ " + blockAddress + ": " + set.getNumAddressRanges());
+		}
+		int length = (int) set.getMaxAddress().subtract(blockAddress) + 1;
 
 		String permissions = (String) result.get("kind");
 		if (permissions == null) {
@@ -103,37 +103,23 @@ public class MemoryMapSarifMgr extends SarifMgr {
 
 		String comment = (String) result.get("comment");
 		String type = (String) result.get("type");
-		String loc = (String) result.get("location");
+		String loc = (String) result.get("location"); // location == position of the bytes w/i file (file::pos)
-		String overlayName = (String) result.get("overlaySpace");
+		// TODO: Explore the possibility of using FileBytes in the future?
-
-		Address blockAddress = addr;
-		if (overlayName != null) {
-			AddressSpace addressSpace = program.getAddressFactory().getAddressSpace(overlayName);
-			if (addressSpace == null) {
-				try {
-					addressSpace = program.createOverlaySpace(overlayName, addr.getAddressSpace());
-				} catch (IllegalStateException | DuplicateNameException | InvalidNameException | LockException e) {
-					throw new RuntimeException("Attempt to create " + overlayName + " failed!");
-				}
-			} else if (!addressSpace.isOverlaySpace()) {
-				throw new RuntimeException(overlayName + " is not a valid overlay space!");
-			}
-			blockAddress = ((OverlayAddressSpace) addressSpace).getAddressInThisSpaceOnly(addr.getOffset());
-		}
 
 		try {
-			byte[] bytes = new byte[length];
-			Arrays.fill(bytes, (byte) 0xff);
 			MemoryBlock block = null;
-			if (type.equals("DEFAULT") && (loc != null)) {
+			if (type.equals("DEFAULT")) {
-				Address startAddr = addr;
+				if (loc == null) {
+					block = MemoryBlockUtils.createUninitializedBlock(program, false, name, blockAddress, length,
+							comment, null, r, w, x, log);
+				} else {
 					String[] split = loc.split(":");
 					String fileName = split[0];
 					int fileOffset = Integer.parseInt(split[1]);
-				setData(bytes, (int) startAddr.subtract(addr), directory, fileName, fileOffset, length, log);
+					byte[] bytes = setData(directory, fileName, fileOffset, length, log);
-
 					block = MemoryBlockUtils.createInitializedBlock(program, false, name, blockAddress,
 							new ByteArrayInputStream(bytes), bytes.length, comment, null, r, w, x, log, monitor);
+				}
 			} else if (type.equals("BIT_MAPPED")) {
 				Address sourceAddr = factory.getAddress(loc);
 				block = MemoryBlockUtils.createBitMappedBlock(program, name, blockAddress, sourceAddr, length, comment,
@@ -143,8 +129,7 @@ public class MemoryMapSarifMgr extends SarifMgr {
 				block = MemoryBlockUtils.createByteMappedBlock(program, name, blockAddress, sourceAddr, length, comment,
 						comment, r, w, x, false, log);
 			} else {
-				block = MemoryBlockUtils.createUninitializedBlock(program, false, name, blockAddress, length, comment,
+				throw new RuntimeException("Unexpected type value - " + type);
-						null, r, w, x, log);
 			}
 			if (block != null) {
 				block.setVolatile(isVolatile);
@@ -156,11 +141,12 @@ public class MemoryMapSarifMgr extends SarifMgr {
 		}
 	}
 
-	private void setData(byte[] bytes, int offset, String directory, String fileName, int fileOffset, int length,
+	private byte[] setData(String directory, String fileName, int fileOffset, int length,
 			MessageLog log) throws IOException {
+		byte[] bytes = new byte[length];
+		Arrays.fill(bytes, (byte) 0xff);
 		File f = new File(directory, fileName);
-		RandomAccessFile binfile = new RandomAccessFile(f, "r");
+		try (RandomAccessFile binfile = new RandomAccessFile(f, "r")) {
-		try {
 			int pos = 0;
 			while (pos < length) {
 				int readLen = (512 * 1024);
@@ -168,16 +154,16 @@ public class MemoryMapSarifMgr extends SarifMgr {
 					readLen = length - pos;
 				}
 				binfile.seek(fileOffset + pos);
-				readLen = binfile.read(bytes, offset + pos, readLen);
+				readLen = binfile.read(bytes, pos, readLen);
 				if (readLen <= 0) {
 					break;
 				}
 				pos += readLen;
 			}
 		} catch (IndexOutOfBoundsException e) {
-			log.appendMsg("Invalid bin file offset " + offset + " with length " + length);
+			log.appendMsg("Read exceeded array length " + length);
 		}
-		binfile.close();
+		return bytes;
 	}
 
 	/////////////////////////////
@@ -188,8 +174,6 @@ public class MemoryMapSarifMgr extends SarifMgr {
 			throws IOException, CancelledException {
 		monitor.setMessage("Writing MEMORY MAP ...");
 
-		bf = isWriteContents ? new MemoryMapBytesFile(program, filePath) : null;
-
 		List<Pair<AddressRange, MemoryBlock>> request = new ArrayList<>();
 		AddressRangeIterator iter = addrs.getAddressRanges();
 		while (iter.hasNext()) {
@@ -203,12 +187,15 @@ public class MemoryMapSarifMgr extends SarifMgr {
 			}
 		}
 
+		try {
+			bf = isWriteContents ? new MemoryMapBytesFile(program, filePath) : null;
 			writeAsSARIF(request, bf, isWriteContents, results);
-
+		} finally {
 			if (isWriteContents) {
 				bf.close();
 			}
 		}
+	}
 
 	public static void writeAsSARIF(List<Pair<AddressRange, MemoryBlock>> request, MemoryMapBytesFile bytes,
 			boolean isWriteContents, JsonArray results) throws IOException {

Ghidra/Features/Sarif/src/main/java/sarif/managers/SarifMgr.java

@@ -22,24 +22,13 @@ import java.util.Map;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-import com.contrastsecurity.sarif.ArtifactLocation;
-import com.contrastsecurity.sarif.Location;
-import com.contrastsecurity.sarif.PhysicalLocation;
-import com.google.gson.JsonArray;
-import com.google.gson.JsonObject;
-
 import ghidra.app.util.NamespaceUtils;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressFactory;
 import ghidra.program.model.address.AddressFormatException;
 import ghidra.program.model.address.AddressOverflowException;
-import ghidra.program.model.address.AddressRange;
-import ghidra.program.model.address.AddressRangeImpl;
-import ghidra.program.model.address.AddressRangeIterator;
 import ghidra.program.model.address.AddressSet;
-import ghidra.program.model.address.AddressSetView;
-import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.listing.Function;
 import ghidra.program.model.listing.Listing;
 import ghidra.program.model.listing.Program;
@@ -52,6 +41,7 @@ import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.exception.InvalidInputException;
 import ghidra.util.task.TaskMonitor;
 import sarif.SarifProgramOptions;
+import sarif.SarifUtils;
 
 public abstract class SarifMgr {
 
@@ -89,89 +79,8 @@ public abstract class SarifMgr {
 		columnKeys.put("typeLocation", true);
 	}
 
-	protected void writeLocation(JsonObject result, Address start, Address end) {
-		JsonArray locs = new JsonArray();
-		result.add("locations", locs);
-		JsonObject element = new JsonObject();
-		locs.add(element);
-		JsonObject ploc = new JsonObject();
-		element.add("physicalLocation", ploc);
-		JsonObject address = new JsonObject();
-		ploc.add("address", address);
-		address.addProperty("absoluteAddress", start.getOffset());
-		if (end != null) {
-			address.addProperty("length", end.subtract(start) + 1);
-			if (!start.getAddressSpace().equals(program.getAddressFactory().getDefaultAddressSpace())) {
-				JsonObject artifact = new JsonObject();
-				ploc.add("artifactLocation", artifact);
-				artifact.addProperty("uri", start.toString());
-			}
-		}
-	}
-
-	protected void writeLocations(JsonObject result, AddressSetView set) {
-		JsonArray locs = new JsonArray();
-		result.add("locations", locs);
-		AddressRangeIterator addressRanges = set.getAddressRanges();
-		while (addressRanges.hasNext()) {
-			JsonObject element = new JsonObject();
-			locs.add(element);
-			AddressRange next = addressRanges.next();
-			JsonObject ploc = new JsonObject();
-			element.add("physicalLocation", ploc);
-			JsonObject address = new JsonObject();
-			ploc.add("address", address);
-			address.addProperty("absoluteAddress", next.getMinAddress().getOffset());
-			address.addProperty("length", next.getLength());
-			Address minAddress = next.getMinAddress();
-			if (!minAddress.getAddressSpace().equals(program.getAddressFactory().getDefaultAddressSpace())) {
-				JsonObject artifact = new JsonObject();
-				ploc.add("artifactLocation", artifact);
-				artifact.addProperty("uri", minAddress.toString());
-			}
-		}
-	}
-
-	@SuppressWarnings("unchecked")
 	protected AddressSet getLocations(Map<String, Object> result, AddressSet set) throws AddressOverflowException {
-		if (set == null) {
+		return SarifUtils.getLocations(result, program, set);
-			set = new AddressSet();
-		}
-		AddressFactory af = program.getAddressFactory();
-		AddressSpace space = af.getDefaultAddressSpace();
-		String namespace = (String) result.get("location");
-		if (namespace != null) {
-			boolean isExternal = namespace.contains("<EXTERNAL>");
-			if (isExternal) {
-				space = af.getAddressSpace("EXTERNAL");
-			}
-		}
-		String ospace = (String) result.get("overlayedSpace");
-		if (ospace != null) {
-			space = af.getAddressSpace(ospace);
-		}
-		List<Location> locations = (List<Location>) result.get("Locations");
-		if (locations == null) {
-			return set;
-		}
-		for (Location location : locations) {
-			PhysicalLocation physicalLocation = location.getPhysicalLocation();
-			Object addr = physicalLocation.getAddress().getAbsoluteAddress();
-			Address address = longToAddress(space, addr);
-			long len = (long) physicalLocation.getAddress().getLength();
-			ArtifactLocation artifact = physicalLocation.getArtifactLocation();
-			if (artifact != null) {
-				String uri = artifact.getUri();
-				if (uri != null) {
-					Address test = program.getAddressFactory().getAddress(uri);
-					if (test != null) {
-						address = test;
-					}
-				}
-			}
-			set.add(new AddressRangeImpl(address, len));
-		}
-		return set;
 	}
 
 	protected Address getLocation(Map<String, Object> result) throws AddressOverflowException {
@@ -259,10 +168,7 @@ public abstract class SarifMgr {
 		}
 		Address addr = factory.getAddress(addrString);
 		if (addr == null) {
-			int index = addrString.indexOf("::");
+			throw new RuntimeException("Error converting "+addrString+" to address");
-			if (index > 0) {
-				addr = factory.getAddress(addrString.substring(index + 2));
-			}
 		}
 		return addr;
 	}
@@ -313,11 +219,4 @@ public abstract class SarifMgr {
 		return unescapedStr;
 	}
 
-	public Address longToAddress(AddressSpace space, Object addr) {
-		if (addr instanceof Long) {
-			return space.getAddress((Long) addr);
-		}
-		return space.getAddress((Integer) addr);
-	}
-
 }

Ghidra/Features/Sarif/src/main/java/sarif/managers/SymbolTableSarifMgr.java

@@ -80,6 +80,7 @@ public class SymbolTableSarifMgr extends SarifMgr {
 			String type = (String) result.get("type");
 
 			boolean isPrimary = (boolean) result.get("primary");
+			boolean isPinned = (boolean) result.get("pinned");
 			String sourceTypeString = (String) result.get("sourceType");
 			SourceType sourceType = getSourceType(sourceTypeString);
 
@@ -134,6 +135,9 @@ public class SymbolTableSarifMgr extends SarifMgr {
 				if (s != null && isPrimary && overwritePrimary) {
 					s.setPrimary();
 				}
+				if (isPinned) {
+					s.setPinned(true);
+				}
 			}
 		} catch (Exception e) {
 			log.appendException(e);