NSA, Ghidra und Einhörner

Auftakt

public class A {
  private String value;
  public A(String val) { // V6022
    this.value = value;
  }
  public int hashCode() {
    return value.hashCode(); // NullPointerException
  }
}

Fragment 1: fehlerhafte Validierung

private boolean parseDataTypeTextEntry()
throws InvalidDataTypeException {
  ...
  try {
    newDataType = parser.parse(selectionField.getText(),
                               getDataTypeRootForCurrentText());
  }
  catch (CancelledException e) {
    return false;
  }
  if (newDataType != null) {
    if (maxSize >= 0
        && newDataType.getLength() > newDataType.getLength()) { // <=
      throw new InvalidDataTypeException("data-type larger than "
                                         + maxSize + " bytes");
    }
    selectionField.setSelectedValue(newDataType);
    return true;
  }
  return false;
}

public class GuidUtil {
  ...
  public static GuidInfo parseLine(...) {
    ...
    long[] data = new long[4];
    ...
    if (isOK(data)) {
      if (!hasVersion) {
        return new GuidInfo(guidString, name, guidType);
      }
      return new VersionedGuidInfo(guidString, version, name, guidType);
    }
    return null;
  }
  ...
  private static boolean isOK(long[] data) {
    for (int i = 0; i < data.length; i++) {
      if ((data[i] != 0) || (data[i] != 0xFFFFFFFFL)) { // <=
        return true;
      }
    }
    return false;
  }
  ...
}

private static boolean isOK(long[] data) {
  for (int i = 0; i < data.length; i++) {
    if ((data[i] == 0) || (data[i] == 0xFFFFFFFFL)) {
      return false;
    }
  }
  return true;
}

Fragment 2: Ausnahmen ausblenden

public void putByte(long offsetInMemBlock, byte b)
throws MemoryAccessException, IOException {
  long offsetInSubBlock = offsetInMemBlock - subBlockOffset;
  try {
    if (ioPending) {
      new MemoryAccessException("Cyclic Access"); // <=
    }
    ioPending = true;
    doPutByte(mappedAddress.addNoWrap(offsetInSubBlock / 8),
              (int) (offsetInSubBlock % 8), b);
  }
  catch (AddressOverflowException e) {
    new MemoryAccessException("No memory at address"); // <=
  }
  finally {
    ioPending = false;
  }
}

• BitMappedSubMemoryBlock.java: Zeilen 77, 99, 106, 122
• ByteMappedSubMemoryBlock.java: Zeilen 52, 73, 92, 114

Fragment 3: Minenfeld

private void processSelection(OptionsTreeNode selectedNode) {
  if (selectedNode == null) {
    setViewPanel(defaultPanel, selectedNode); // <=
    return;
  }
  ...
}
private void setViewPanel(JComponent component, OptionsTreeNode selectedNode) {
  ...
  setHelpLocation(component, selectedNode);
  ...
}
private void setHelpLocation(JComponent component, OptionsTreeNode node) {
  Options options = node.getOptions();
  ...
}

Fragment 4: Automatische Vervollständigung für das Böse

public static final int[] UNSUPPORTED_OPCODES_LIST = { ... };
public static final Set<Integer> UNSUPPORTED_OPCODES = new HashSet<>();

static {
  for (int opcode : UNSUPPORTED_OPCODES) {
    UNSUPPORTED_OPCODES.add(opcode);
  }
}

Fragment 5: da ist alles

public void setValueAt(Object aValue, int row, int column) {
  ...
  int index = indexOf(newName);
  if (index >= 0) {                  // <=
    Window window = tool.getActiveWindow();
    Msg.showInfo(getClass(), window, "Duplicate Name",
                 "Name already exists: " + newName);
    return;
  }

  ExternalPath path = paths.get(row); // <=
  ...
}
private int indexOf(String name) {
  for (int i = 0; i < paths.size(); i++) {
    ExternalPath path = paths.get(i);
    if (path.getName().equals(name)) {
      return i;
    }
  }
  return 0;
}

• V6007 Der Ausdruck 'index> = 0' ist immer wahr. ExternalNamesTableModel.java:105
• V6019 Unreachable code detected. It is possible that an error is present. ExternalNamesTableModel.java:109

Fragment 6: Schweigen

final static Map<Character, String> DELIMITER_NAME_MAP = new HashMap<>(20);
// Any non-alphanumeric char can be used as a delimiter.
static {
  DELIMITER_NAME_MAP.put(' ', "Space");
  DELIMITER_NAME_MAP.put('~', "Tilde");
  DELIMITER_NAME_MAP.put('`', "Back quote");
  DELIMITER_NAME_MAP.put('@', "Exclamation point");
  DELIMITER_NAME_MAP.put('@', "At sign");
  DELIMITER_NAME_MAP.put('#', "Pound sign");
  DELIMITER_NAME_MAP.put('$', "Dollar sign");
  DELIMITER_NAME_MAP.put('%', "Percent sign");
  ...
}

Fragment 7: Der Rest der Teilung ist immer 0

void setFactorys(FieldFactory[] fieldFactorys,
                 DataFormatModel dataModel, int margin) {
  factorys = new FieldFactory[fieldFactorys.length];

  int x = margin;
  int defaultGroupSizeSpace = 1;
  for (int i = 0; i < factorys.length; i++) {
    factorys[i] = fieldFactorys[i];
    factorys[i].setStartX(x);
    x += factorys[i].getWidth();
    // add in space between groups
    if (((i + 1) % defaultGroupSizeSpace) == 0) { // <=
      x += margin * dataModel.getUnitDelimiterSize();
    }
  }
  width = x - margin * dataModel.getUnitDelimiterSize() + margin;
  layoutChanged();
}

• V6007 Ausdruck '((i + 1)% defaultGroupSizeSpace) == 0' ist immer wahr. ByteViewerLayoutModel.java:66
• V6048 Dieser Ausdruck kann vereinfacht werden. Der Operand 'defaultGroupSizeSpace' in der Operation entspricht 1. ByteViewerLayoutModel.java:66

Fragment 8: gebrochene Validierung, Teil 2

private String parseArrayDimensions(String datatype,
                                    List<Integer> arrayDimensions) {
  String dataTypeName = datatype;
  boolean zeroLengthArray = false;
  while (dataTypeName.endsWith("]")) {
    if (zeroLengthArray) {                   // <=
      return null; // only last dimension may be 0
    }
    int rBracketPos = dataTypeName.lastIndexOf(']');
    int lBracketPos = dataTypeName.lastIndexOf('[');
    if (lBracketPos < 0) {
      return null;
    }
    int dimension;
    try {
      dimension = Integer.parseInt(dataTypeName.substring(lBracketPos + 1,
                                                          rBracketPos));
      if (dimension < 0) {
        return null; // invalid dimension
      }
    }
    catch (NumberFormatException e) {
      return null;
    }
    dataTypeName = dataTypeName.substring(0, lBracketPos).trim();
    arrayDimensions.add(dimension);
  }
  return dataTypeName;
}

try {
  dimension = Integer.parseInt(dataTypeName.substring(lBracketPos + 1,
                                                      rBracketPos));
  if (dimension < 0) {
    return null; // invalid dimension
  } else if (dimension == 0) {
    zeroLengthArray = true;
  }
}
catch (NumberFormatException e) {
  return null;
}

Ein bisschen über den Rest der Warnungen

Fazit

• Toter Code, der höchstwahrscheinlich nach zahlreichen Umgestaltungen erhalten blieb;
• Viele Javadocs sind hoffnungslos veraltet und weisen beispielsweise auf nicht vorhandene Parameter hin.
• Bei Verwendung von IntelliJ IDEA besteht keine Möglichkeit einer bequemen Entwicklung .
• Ein modulares System, das auf Reflexion basiert, erschwert das Navigieren in einem Projekt und das Auffinden von Abhängigkeiten zwischen Komponenten erheblich.