BUG: Stop loc_strnlen reading one byte past the buffer it is given

[gdevenyi]

loc_strnlen() exists to measure a string that need not be terminated,
but it dereferences before it checks the bound:

for( sptr=str, len=0; *sptr && len<maxlen; sptr++, len++)

When no NUL appears in the first maxlen bytes, the last iteration reads
str[maxlen], one byte past the end.

Both callers pass a buffer with no terminator:

• axml_read_buf() is handed a caller's buffer and its length. Any
  XML buffer that is exactly its own length -- an in-memory document,
  or the CIFTI extension of a NIfTI file -- reads past the end.
• axml_read_file() measures what fread() just returned. The read
  fills the whole buffer for every XML file larger than the buffer,
  so a heap over-read happens on each full block, once per
  axml_get_buf_size() bytes of file.

The existing tests do not catch it because their XML is smaller than
the buffer, which leaves the byte after the data inside the allocation.
AddressSanitizer on a buffer sized to its content:

ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 ...
    #0 loc_strnlen afni_xml.c:1058
    #1 axml_read_buf afni_xml.c:282
... is located 0 bytes after 324-byte region

Swapping the two conditions fixes it; the returned length is unchanged
in every case that was already in bounds.

Found by fuzzing axml_read_buf() with clang's libFuzzer under
AddressSanitizer.

---

How to reproduce, and the sanitizer builds used, are in the commit message. Found with clang libFuzzer harnesses over nifti_image_read(), the two header converters, nifti_image_from_ascii() and axml_read_buf(), run under AddressSanitizer, UndefinedBehaviorSanitizer and MemorySanitizer.

One commit on master, independent of my other open PRs. ctest passes on master with this branch alone (343/343, excluding the nifti_c22_copy_image failure that #31 fixes), and on the union of all my branches under ASan+UBSan, TSan and MSan.

[hjmjohnson]

Rebased onto current master. Verified on Ubuntu 24.04 / GCC 13.3, shared, cifti and fsl on: 0 warnings, 345/345 tests, symbol baseline unchanged at 439.

This is the first time the change has been built by CI at all — cifti/ was compiled by no job until #75 added the USE_CIFTI_CODE=ON legs.

Commit message trimmed to the prose budget

The body was 30 lines and carried Co-Authored-By: Claude Opus 5 and a Claude-Session: URL. Both are removed; authorship is unchanged.

What was kept is the part a future reader cannot get from the diff: the mechanism (the loop dereferences before testing the bound, so it reads str[maxlen]), why it is reachable (both callers pass buffers that need not be terminated), and the AddressSanitizer evidence. What was cut: the inlined for loop, the per-caller walkthrough, and the note on how the bug was found.

The test suite cannot catch this, and that is worth recording

As the original commit message observed, the existing tests use XML smaller than the buffer, so the byte after the data still falls inside the allocation and the over-read is invisible.

That means merging this does not make the path covered — it makes it correct. A regression test needs a buffer sized exactly to its contents, which is the same class of fixture the malformed-input corpus would provide. Recorded against the testing-gap notes rather than claimed as tested here.

[hjmjohnson]

The commit messages in this range were rewritten to remove trailers that do not belong in permanent history: Co-Authored-By: naming an AI tool, and Claude-Session: URLs that resolve for nobody. Only messages changed — the tree at the tip of master is byte-identical, and author, committer, and dates are preserved.

This PR's commit on the rewritten master:

• 7476a12456 BUG: Stop loc_strnlen reading one byte past the buffer it is given

The SHA recorded above by GitHub is from the pre-rewrite history and no longer resolves.