search

3dem / relion

Image-processing software for cryo-electron microscopy
https://relion.readthedocs.io/en/latest/
GNU General Public License v2.0
456 stars 203 forks source link

Bug found `void removeQuotes(char **_str)` in src/strings.cpp #1200

Closed Arpan3323 closed 1 week ago

Arpan3323 commented 1 week ago

Hi, I came across this function and noticed that it lacked null checks for _str and further investigation has revealed existence of undefined behavior in this function. https://github.com/3dem/relion/blob/f2e59d6ec61d3f92df31cebbb7402f1012b17a9e/src/strings.cpp#L471

Fuzz testing this function in isolation gave me runtime error: addition of unsigned offset which usually happens when we do lookup in array X using Y (i.e X[Y]) where Y is extremely huge and invalid index. I believe the cause is this line: c = retval[retval.length()-1]; because we do not check if the length is 0 before subtracting 1. I cannot exactly find where this function is being called in the project and it may be dead code that is not used anymore.

I will submit a PR shortly that fixes this issue, makes the function safer for use, and potentially improves efficiency.

Please let me know if you think I am mistaken :)

biochem-fan commented 1 week ago

@Arpan3323 Thank you very much for your report. Indeed this function is not used at all. Could you please send us a PR to remove this function, targeting the ver5.0 branch?

Arpan3323 commented 1 week ago

Absolutely. I will also close #1201 in that case.