search

node-ffi-napi / ref-napi

Turn Buffer instances into "pointers"
MIT License
123 stars 67 forks source link

It seems the writeCString may write over the end of buffer #40

Closed wjzhou closed 4 years ago

wjzhou commented 4 years ago

I'm trying to debug a crash of my application. (Not isolated yet)

When reading the source code, it seems the following function may write over the end of buffer, did I miss anything?

L582 may return a len = size which equal to buffer.length-offset. so at L583, offset+len equal offset+buffer.length-offset = buffer.length

Shouldn't the L581 be const size = buffer.length - offset-1?

https://github.com/node-ffi-napi/ref-napi/blob/7d0d7b24d242372c7557d571cbe2a87ef67ac53d/lib/ref.js#L572-L584

addaleax commented 4 years ago

Thanks for pointing this out! Yes, this looks like a bug. I’ll publish a new release shortly.

addaleax commented 4 years ago

Fwiw, the line in question does not actually perform an OOB write; it throws an exception if offset+len is OOB, so it’s probably not the cause of the crash you were seeing.

addaleax commented 4 years ago

Done in v2.0.4!

wjzhou commented 4 years ago

Thank you very much for the quick fix. And thank you for giving me clues on my situation.

FYI: I believe my problem was in reinterpretUntilZeros, similar to this one: https://github.com/sindresorhus/active-win/pull/65 E.g. I am also using electron and after removing all the reinterpretUntilZeros, my program doesn't see crashes anymore.

Reading the source code of reinterpretUntilZeros, I don't think there is a problem in our end. For now, because I actually know the size of the number of characters inside the buffer, I just use Buffer.toString('ucs2', 0, numOfChars*2).