Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large header extensions can result in failure to parse NIfTI headers #126

Closed
effigies opened this issue Dec 3, 2024 · 0 comments · Fixed by #127
Closed

Large header extensions can result in failure to parse NIfTI headers #126

effigies opened this issue Dec 3, 2024 · 0 comments · Fixed by #127
Labels
bug Something isn't working

Comments

@effigies
Copy link
Contributor

effigies commented Dec 3, 2024

I manually decompressed and loaded the first kB of data from a failing file:

> readHeader(buf.buffer)
Uncaught RangeError: Offset is outside the bounds of the DataView
    at DataView.prototype.getInt32 (<anonymous>)
    at Function.getIntAt (file:///home/chris/.cache/deno/npm/registry.npmjs.org/@bids/nifti-reader-js/0.6.9/dist/src/utilities.js:28:21)
    at Function.getExtensionsAt (file:///home/chris/.cache/deno/npm/registry.npmjs.org/@bids/nifti-reader-js/0.6.9/dist/src/utilities.js:53:31)
    at NIFTI1.readHeader (file:///home/chris/.cache/deno/npm/registry.npmjs.org/@bids/nifti-reader-js/0.6.9/dist/src/nifti1.js:335:53)
    at readHeader (file:///home/chris/.cache/deno/npm/registry.npmjs.org/@bids/nifti-reader-js/0.6.9/dist/src/nifti.js:146:16)
    at <anonymous>:1:22

The relevant lines are here:

https://github.com/bids-standard/NIFTI-Reader-JS/blob/0e406b03775d5687911f9440dffa65d7b0886137/src/nifti1.ts#L386-L412

Because we pass in 540 bytes to allow for NIfTI2, the header parser attempts to parse extensions, and crashes if the extension extends beyond 540 bytes. If we passed 348 bytes in, we would safely ignore the extensions, but break on NIfTI2 images.

const buf = await file.readBytes(1024)
const data = isCompressed(buf.buffer) ? await extract(buf, 540) : buf
const header = readHeader(data.buffer)
if (!header) {
throw { key: 'NIFTI_HEADER_UNREADABLE' }
}

I think probably the right way to fix this is to check if a file is NIfTI 1 or 2 (https://github.com/bids-standard/NIFTI-Reader-JS/blob/0e406b03775d5687911f9440dffa65d7b0886137/src/nifti.ts#L16-L74) and then load the header with the exact number of bytes.

The other approach I can see is to use a try/catch block and fall back to parsing 348 bytes.

Another approach could be to add a flag in NIFTI-Reader-JS to ignore extensions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant