diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/FunctionPlugin.java b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/FunctionPlugin.java index 20ce5b113f..7ccc811d44 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/FunctionPlugin.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/function/FunctionPlugin.java @@ -388,23 +388,7 @@ public class FunctionPlugin extends Plugin implements DataService { Address loc = location.getAddress(); return program.getFunctionManager().getFunctionsOverlapping(new AddressSet(loc, loc)); } - //return an empty iterator.... - return new Iterator<>() { - @Override - public void remove() { - // not supported - } - - @Override - public boolean hasNext() { - return false; - } - - @Override - public Function next() { - return null; - } - }; + return Collections.emptyIterator(); } Function getFunction(ListingActionContext context) { diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/OptionsEditorPanel.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/OptionsEditorPanel.java index ff283697e5..d8d11a4b0f 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/OptionsEditorPanel.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/OptionsEditorPanel.java @@ -247,6 +247,8 @@ public class OptionsEditorPanel extends JPanel { combo.setSelectedItem(space); } combo.addActionListener(e -> { + // called whenever the combobox changes to push the value back to the Option that is + // our 'model' option.setValue(combo.getSelectedItem()); }); return combo; diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/dialog/AskAddrDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/dialog/AskAddrDialog.java index 04f7e3cd4d..c6361928f8 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/dialog/AskAddrDialog.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/dialog/AskAddrDialog.java @@ -26,7 +26,6 @@ import docking.widgets.label.GLabel; import ghidra.app.util.AddressInput; import ghidra.program.model.address.Address; import ghidra.program.model.address.AddressFactory; -import ghidra.util.SystemUtilities; public class AskAddrDialog extends DialogComponentProvider { private boolean isCanceled; @@ -54,8 +53,7 @@ public class AskAddrDialog extends DialogComponentProvider { addCancelButton(); setDefaultButton(okButton); - SystemUtilities.runSwingNow( - () -> DockingWindowManager.showDialog(null, AskAddrDialog.this)); + DockingWindowManager.showDialog(null, this); } @Override diff --git a/Ghidra/Features/Base/src/test/java/ghidra/app/util/html/DataTypeDifferTest.java b/Ghidra/Features/Base/src/test/java/ghidra/app/util/html/DataTypeDifferTest.java index 130357834e..ac7dd9824b 100644 --- a/Ghidra/Features/Base/src/test/java/ghidra/app/util/html/DataTypeDifferTest.java +++ b/Ghidra/Features/Base/src/test/java/ghidra/app/util/html/DataTypeDifferTest.java @@ -26,7 +26,7 @@ import javax.swing.*; import org.junit.Assert; import org.junit.Test; -import docking.widgets.label.GDLabel; +import docking.widgets.label.GDHtmlLabel; import ghidra.app.util.html.diff.*; public class DataTypeDifferTest { @@ -491,7 +491,7 @@ public class DataTypeDifferTest { JPanel rightPanel = new JPanel(new BorderLayout()); StringBuffer buffy1 = new StringBuffer(htmlLeft); - JLabel rightLabel = new GDLabel(); + JLabel rightLabel = new GDHtmlLabel(); rightLabel.setOpaque(true); rightLabel.setBackground(Color.WHITE); rightLabel.setVerticalAlignment(SwingConstants.TOP); @@ -499,7 +499,7 @@ public class DataTypeDifferTest { JPanel leftPanel = new JPanel(new BorderLayout()); StringBuffer buffy2 = new StringBuffer(htmlRight); - JLabel leftLabel = new GDLabel(); + JLabel leftLabel = new GDHtmlLabel(); leftLabel.setOpaque(true); leftLabel.setBackground(Color.WHITE); leftLabel.setVerticalAlignment(SwingConstants.TOP); diff --git a/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ByteViewerPlugin.java b/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ByteViewerPlugin.java index 8f86b9de6d..65e36fcfe2 100644 --- a/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ByteViewerPlugin.java +++ b/Ghidra/Features/ByteViewer/src/main/java/ghidra/app/plugin/core/byteviewer/ByteViewerPlugin.java @@ -67,8 +67,7 @@ public class ByteViewerPlugin extends Plugin { private ProgramByteViewerComponentProvider connectedProvider; - private List disconnectedProviders = - new ArrayList<>(); + private List disconnectedProviders = new ArrayList<>(); public ByteViewerPlugin(PluginTool tool) { super(tool); @@ -336,7 +335,7 @@ public class ByteViewerPlugin extends Plugin { /** * Set the status info on the tool. * - * @param msg non-html text to display + * @param msg text to display * @param provider not used */ void setStatusMessage(String msg, ComponentProvider provider) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java b/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java index 3106447a36..0de27260cf 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/DockingUtils.java @@ -20,13 +20,76 @@ import java.awt.event.*; import javax.swing.*; import javax.swing.plaf.basic.BasicSeparatorUI; +import javax.swing.table.TableCellRenderer; import javax.swing.text.Document; import javax.swing.text.JTextComponent; +import javax.swing.tree.DefaultTreeCellRenderer; +import javax.swing.tree.TreeCellRenderer; import javax.swing.undo.UndoableEdit; +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; +import docking.widgets.combobox.GComboBox; +import docking.widgets.combobox.GhidraComboBox; +import docking.widgets.label.*; +import docking.widgets.list.GList; +import docking.widgets.list.GListCellRenderer; +import docking.widgets.table.GTableCellRenderer; +import docking.widgets.tree.support.GTreeRenderer; import ghidra.docking.util.DockingWindowsLookAndFeelUtils; +import ghidra.util.HTMLUtilities; import resources.ResourceManager; +/** + *

Notes about how to use HTML safely:

+ * Java's built-in HTML rendering in UI components is very useful, but can also introduce security + * issues when a hostile actor is providing the text strings that are being rendered. + *

+ * Before using a native Java UI component, search for a corresponding 'G'hidra component, and + * if possible choose the non-HTML version of that component (if available). + *

+ * For instance, instead of using {@link JLabel}, use either {@link GLabel} or {@link GHtmlLabel} + * (and their variants). + *

+ * (native JLabel, JCheckbox, etc, usage is actually disallowed in the Ghidra project) + *

+ * When using a UI component that is HTML enabled, care must be used when constructing the text + * that is being rendered. + *

+ * During string-building or concatenation, appending a non-literal string value (ie. + * {@code "Hello " + getFoo();} ), the non-literal string value should be escaped using + * {@link HTMLUtilities#escapeHTML(String)} (ie. {@code "Hello " + HTMLUtilities.escapeHTML(getFoo());}. + *

+ * Of course, there are exceptions to every rule, and if the string value can be definitely be + * traced to its source and there are no user-supplied origins, the HTML escaping can be skipped. + *

+ * Note: just using a UI component that is HTML enabled does not mean that it will treat its + * text as HTML text. If you need to HTML escape any values that are being fed to the component, you + * need to force the HTML mode 'on' by pre-pending a "<HTML>" at the beginning of the string. + * If you fail to do this, the escaped substrings will look wrong because any '<' and '>' chars + * (and others) in the substring will be mangled when rendered in plain-text mode. + *

+ * When working with plain text, try to avoid allowing a user supplied string being the first + * value of text that could be fed to a UI component. This will prevent the possibly hostile + * string from having a leading HTML start tag. + * (ie. when displaying an error to the user about a bad file, don't put the filename + * value at the start of the string, but instead put a quote or some other delimiter to prevent + * html mode). + *

+ *

Recommended Ghidra UI Components:

+ *

+ * + * + * + * + * + * + * + * + * + * + *
Native ComponentRecommended Component
{@link JLabel}{@link GLabel}
{@link GDLabel}
{@link GHtmlLabel}
{@link GDHtmlLabel}
{@link GIconLabel}
{@link JCheckBox}{@link GCheckBox}
{@link GHtmlCheckBox}
{@link JComboBox}{@link GComboBox}
{@link GhidraComboBox}
{@link JList}{@link GList}
{@link ListCellRenderer}
{@link DefaultListCellRenderer}		{@link GListCellRenderer}
{@link TableCellRenderer}{@link GTableCellRenderer}
{@link TreeCellRenderer}
{@link DefaultTreeCellRenderer}		{@link GTreeRenderer}
{@link DnDTreeCellRenderer}
{@link JButton}???tbd???
+ */ public class DockingUtils { // taken from BasicHTML.htmlDisable, which is private public static final String HTML_DISABLE_STRING = "html.disable"; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java b/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java index d56affdf1d..a24a375469 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/DockingWindowManager.java @@ -1904,7 +1904,7 @@ public class DockingWindowManager implements PropertyChangeListener, Placeholder /** * Set the status text in the active component window. - * @param text non-html status text + * @param text status text */ public void setStatusText(String text) { if (root != null) { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GCheckBox.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GCheckBox.java index 336f517f66..c257f8b2d0 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GCheckBox.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GCheckBox.java @@ -20,7 +20,14 @@ import javax.swing.*; import docking.DockingUtils; /** - * A {@link JCheckBox} that has HTML rendering disabled + * A {@link JCheckBox} that has HTML rendering disabled. + *

+ * See also: + * + * + * + * + *
ClassHTML renderingDescription
{@link GCheckBox}NOHTML disabled JCheckBox
{@link GHtmlCheckBox}YESHTML allowed JCheckBox
*/ public class GCheckBox extends JCheckBox { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GHtmlCheckBox.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GHtmlCheckBox.java index 4bdabc2d81..5d12b68043 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GHtmlCheckBox.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/checkbox/GHtmlCheckBox.java @@ -19,7 +19,14 @@ import javax.swing.*; /** * A {@link JCheckBox} that allows HTML rendering. - */ + *

+ * See also: + * + * + * + * + *
ClassHTML renderingDescription
{@link GCheckBox}NOHTML disabled JCheckBox
{@link GHtmlCheckBox}YESHTML allowed JCheckBox
+*/ public class GHtmlCheckBox extends JCheckBox { /** diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDHtmlLabel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDHtmlLabel.java index e0d9b8eea4..14728b2e42 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDHtmlLabel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDHtmlLabel.java @@ -17,8 +17,11 @@ package docking.widgets.label; import javax.swing.*; +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; + /** - * A 'dynamic' label with HTML rendering allowed. + * A 'dynamic' label (the text can be changed), with HTML rendering allowed. *

* See also: * @@ -28,6 +31,9 @@ import javax.swing.*; * * * + * + * + * *
{@link GHtmlLabel}ImmutableYESHtml unchangeable label
{@link GDHtmlLabel}MutableYESHtml changeable label
{@link GIconLabel}N/ANOLabel that only has an icon image, no text
Other components of note:
{@link GCheckBox}NONon-html checkbox
{@link GHtmlCheckBox}YESHtml checkbox
*/ public class GDHtmlLabel extends JLabel { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDLabel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDLabel.java index 40b89ce647..707c266a38 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDLabel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GDLabel.java @@ -18,9 +18,11 @@ package docking.widgets.label; import javax.swing.*; import docking.DockingUtils; +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; /** - * A 'dynamic' label with HTML rendering disabled. + * A 'dynamic' label (the text can be changed), with HTML rendering disabled. *

* See also: * @@ -30,6 +32,9 @@ import docking.DockingUtils; * * * + * + * + * *
{@link GHtmlLabel}ImmutableYESHtml unchangeable label
{@link GDHtmlLabel}MutableYESHtml changeable label
{@link GIconLabel}N/ANOLabel that only has an icon image, no text
Other components of note:
{@link GCheckBox}NONon-html checkbox
{@link GHtmlCheckBox}YESHtml checkbox
*/ public class GDLabel extends JLabel { diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GHtmlLabel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GHtmlLabel.java index 3fbab28d5c..16fbc62320 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GHtmlLabel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GHtmlLabel.java @@ -17,10 +17,15 @@ package docking.widgets.label; import javax.swing.*; +import org.apache.commons.lang3.StringUtils; + +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; import ghidra.util.Msg; +import utilities.util.reflection.ReflectionUtilities; /** - * An immutable label with HTML rendering allowed. + * An immutable label (the text can NOT be changed), with HTML rendering allowed. *

* See also: * @@ -30,6 +35,9 @@ import ghidra.util.Msg; * * * + * + * + * *
{@link GHtmlLabel}ImmutableYESHtml unchangeable label
{@link GDHtmlLabel}MutableYESHtml changeable label
{@link GIconLabel}N/ANOLabel that only has an icon image, no text
Other components of note:
{@link GCheckBox}NONon-html checkbox
{@link GHtmlCheckBox}YESHtml checkbox
*/ public class GHtmlLabel extends JLabel { @@ -108,9 +116,11 @@ public class GHtmlLabel extends JLabel { @Deprecated @Override public void setText(String text) { - if (getText() != null && !getText().isEmpty()) { - Msg.warn(this, "Trying to set text on an immutable label! Current text: [" + - getText() + "], new text: [" + text + "]", new Throwable()); + if (StringUtils.isEmpty(getText())) { + Msg.warn(this, + "Trying to set text on an immutable label! Current text: [" + getText() + + "], new text: [" + text + "]", + ReflectionUtilities.createJavaFilteredThrowable()); return; } super.setText(text); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GIconLabel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GIconLabel.java index 15ffba9606..a405704fd9 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GIconLabel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GIconLabel.java @@ -17,7 +17,12 @@ package docking.widgets.label; import javax.swing.*; +import org.apache.commons.lang3.StringUtils; + +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; import ghidra.util.Msg; +import utilities.util.reflection.ReflectionUtilities; /** * A label that only contains an image and no text. @@ -30,6 +35,9 @@ import ghidra.util.Msg; * {@link GHtmlLabel}ImmutableYESHtml unchangeable label * {@link GDHtmlLabel}MutableYESHtml changeable label * {@link GIconLabel}N/ANOLabel that only has an icon image, no text + * Other components of note: + * {@link GCheckBox}NONon-html checkbox + * {@link GHtmlCheckBox}YESHtml checkbox * */ public class GIconLabel extends GLabel { @@ -88,9 +96,9 @@ public class GIconLabel extends GLabel { @Deprecated @Override public void setText(String text) { - if (text != null && !text.isEmpty()) { + if (!StringUtils.isEmpty(text)) { Msg.warn(this, "Trying to set text on an icon label! New text: [" + text + "]", - new Throwable()); + ReflectionUtilities.createJavaFilteredThrowable()); return; } super.setText(text); diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GLabel.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GLabel.java index 14289e5cd7..4df8631f47 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GLabel.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/label/GLabel.java @@ -17,11 +17,16 @@ package docking.widgets.label; import javax.swing.*; +import org.apache.commons.lang3.StringUtils; + import docking.DockingUtils; +import docking.widgets.checkbox.GCheckBox; +import docking.widgets.checkbox.GHtmlCheckBox; import ghidra.util.Msg; +import utilities.util.reflection.ReflectionUtilities; /** - * An immutable label with HTML rendering disabled. + * An immutable label (the text can NOT be changed), with HTML rendering disabled. *

* See also: * @@ -31,6 +36,9 @@ import ghidra.util.Msg; * * * + * + * + * *
{@link GHtmlLabel}ImmutableYESHtml unchangeable label
{@link GDHtmlLabel}MutableYESHtml changeable label
{@link GIconLabel}N/ANOLabel that only has an icon image, no text
Other components of note:
{@link GCheckBox}NONon-html checkbox
{@link GHtmlCheckBox}YESHtml checkbox
*/ public class GLabel extends JLabel { @@ -100,9 +108,6 @@ public class GLabel extends JLabel { init(); } - //--------------------------------------------------------------------------------------------- - // Protected ctors for derived classes - //--------------------------------------------------------------------------------------------- /** * * @param image icon to display @@ -126,8 +131,6 @@ public class GLabel extends JLabel { init(); } - //--------------------------------------------------------------------------------------------- - private void init() { DockingUtils.turnOffHTMLRendering(this); } @@ -145,20 +148,29 @@ public class GLabel extends JLabel { @Deprecated @Override public void setText(String text) { - if (getText() != null && !getText().isEmpty()) { - Msg.warn(this, "Trying to set text on an immutable label! Current text: [" + - getText() + "], new text: [" + text + "]", new Throwable()); + if (!StringUtils.isEmpty(getText())) { + Msg.warn(this, + "Trying to set text on an immutable label! Current text: [" + getText() + + "], new text: [" + text + "]", + ReflectionUtilities.createJavaFilteredThrowable()); return; } warnAboutHtmlText(text); super.setText(text); } + /** + * Helper function that logs a warning about a string text that looks like it has HTML text. + *

+ * Use this when working with a string in a label that has already disabled HTML rendering. + *

+ * @param text string to test for HTML and warn about + */ public static void warnAboutHtmlText(String text) { // #ifdef still_finding_html_labels_in_our_huge_codebase - if (text != null && (text.startsWith("") || text.startsWith(""))) { + if (StringUtils.startsWithIgnoreCase(text, "")) { Msg.warn(GLabel.class, "HTML text detected in non-HTML component: " + text, - new Throwable()); + ReflectionUtilities.createJavaFilteredThrowable()); } // #endif } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GList.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GList.java index 1e2dd1c75e..e0d4233c4b 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GList.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GList.java @@ -27,8 +27,13 @@ import docking.widgets.table.GTable; /** * A sub-class of JList that provides an auto-lookup feature. + *

* The user can begin typing the first few letters of a desired * list element and the selection will automatically navigate to it. + *

+ * HTML rendering is disabled by default. + *

+ * */ public class GList extends JList { private static final long serialVersionUID = 1L; diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GListCellRenderer.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GListCellRenderer.java index 180c653b40..034c455fb3 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GListCellRenderer.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/list/GListCellRenderer.java @@ -24,6 +24,8 @@ import docking.widgets.AbstractGCellRenderer; /** * Provides a common implementation of a list renderer, for use in both JList and JComboBox. + *

+ * HTML rendering defaults to disabled. See {@link #setHTMLRenderingEnabled(boolean)}. * * @param the element-type this list models. */ diff --git a/Ghidra/Framework/Generic/src/main/java/resources/ResourceManager.java b/Ghidra/Framework/Generic/src/main/java/resources/ResourceManager.java index b08c9528e1..0883e8803f 100644 --- a/Ghidra/Framework/Generic/src/main/java/resources/ResourceManager.java +++ b/Ghidra/Framework/Generic/src/main/java/resources/ResourceManager.java @@ -539,7 +539,9 @@ public class ResourceManager { } /** - * Load the image specified by filename; returns null if problems occur trying to load the file + * Load the image specified by filename; returns the default bomb icon + * if problems occur trying to load the file. + *

* * @param filename name of file to load, e.g., "images/home.gif" * @return the image icon stored in the bytes @@ -574,6 +576,22 @@ public class ResourceManager { return getDefaultIcon(); } + /** + * Load the images specified by filenames; substitutes the default bomb icon + * if problems occur trying to load an individual file. + *

+ * @param filenames vararg list of string filenames (ie. "images/home.gif") + * @return list of ImageIcons with each image, problem / missing images replaced with + * the default icon. + */ + public static List loadImages(String... filenames) { + List results = new ArrayList<>(filenames.length); + for (String filename : filenames) { + results.add(loadImage(filename)); + } + return results; + } + /** * A convenience method to force the image denoted by filename to be read * from disk and to not use the cached version @@ -690,12 +708,4 @@ public class ResourceManager { testSearchPaths = results; return testSearchPaths; } - - public static List loadImages(String... filenames) { - List results = new ArrayList<>(filenames.length); - for (String filename : filenames) { - results.add(loadImage(filename)); - } - return results; - } }