Reject a BAM record whose read-name length is 0 when it is decoded

CHANGELOG.md

@@ -10,6 +10,16 @@ early infrastructure for a plugin-based codec framework and resource bundles.
 
 ---
 
+## 6.0.1
+
+Patch release on the 6.0 line.
+
+### Bug fixes
+
+- **A BAM record with a read-name length of 0 is rejected when it is decoded** ([#1948](https://github.com/samtools/htsjdk/issues/1948)).  The length counts the NUL that ends the name, so 0 is never valid.  Such a record used to decode, then fail on `getReadName()` with `StringIndexOutOfBoundsException` and decode its CIGAR, bases and qualities from the wrong bytes; it now fails at once with `SAMFormatException`, as htslib rejects it.
+
+---
+
 ## 6.0.0
 
 Major release.  It includes every change in 5.0.2.  Before upgrading, read [Upgrading from 5.x](#upgrading-from-5x); to explain differences from files 5.x wrote, see [Output that differs from 5.x](#output-that-differs-from-5x).

src/main/java/htsjdk/samtools/BAMRecordCodec.java

@@ -280,6 +280,10 @@ public SAMRecord decode() {
         final int referenceID = this.binaryCodec.readInt();
         final int coordinate = this.binaryCodec.readInt() + 1;
         final short readNameLength = this.binaryCodec.readUByte();
+        if (readNameLength == 0) {
+            // The length includes the NUL that ends the name, so even an empty name has a length of 1
+            throw new SAMFormatException("Invalid read name length: 0");
+        }
         final short mappingQuality = this.binaryCodec.readUByte();
         final int bin = this.binaryCodec.readUShort();
         final int cigarLen = this.binaryCodec.readUShort();

src/test/java/htsjdk/samtools/BAMRecordCodecTest.java

@@ -109,4 +109,15 @@ public void testCigarTooLongForBamIsStoredAsUnsignedIntArrayInCgTag() {
         Assert.assertTrue(encoded.contains("CGBI"), "CG tag is not stored as B:I");
         Assert.assertFalse(encoded.contains("CGBi"), "CG tag is stored as B:i");
     }
+
+    @Test(expectedExceptions = SAMFormatException.class)
+    public void testZeroReadNameLengthIsRejectedAtDecode() {
+        final SAMFileHeader header = new SAMFileHeader();
+        final byte[] bytes = encode(header, unmappedRecord(header, "r1")).getBytes(StandardCharsets.ISO_8859_1);
+        // l_read_name follows block_size, refID and pos; it counts the NUL that ends the name, so it can't be 0
+        bytes[12] = 0;
+        final BAMRecordCodec codec = new BAMRecordCodec(header);
+        codec.setInputStream(new ByteArrayInputStream(bytes));
+        codec.decode();
+    }
 }