search

onetrueawk / awk

One true awk
Other
1.98k stars 159 forks source link

Avoid undefined behavior, passing memcpy() a NULL destination pointer. #155

Closed millert closed 1 year ago

millert commented 2 years ago

The C standard says that passing a NULL pointer results in undefined behavior, even if the byte count is 0. This quiets the warning from UBSAN for, e.g. 'BEGIN { print foo[0] "" }' As pointed out by @mpinjr there is also a use-after-free where y=execute(a[1]) could delete x so we need to free x before executing y.

mpinjr commented 2 years ago

Hi, Todd:

There's a use-after-free issue with tempfree(x) in the master branch and in your diffs: y = execute(a[1]) may have deleted x.

In your second diff, memcpy(s, x->sval, n1) is afflicted as well.

I think your first version modified to call tempfree(x) before y=execute(a[1]) would be correct.

valgrind detects the invalid reads in the following program (which should return str1str2):

function f() { delete a[0]; return "str2" }
BEGIN { a[0] = "str1"; print a[0] f() }

Take care, Miguel

millert commented 2 years ago

Thanks @mpinjr, I didn't notice the existing use-after-free.

mpinjr commented 2 years ago

beca83b looks good to me. I can confirm that both the test suite and my valgrind test pass.

plan9 commented 1 year ago

sorry for the delay in getting back to this. the issue here was not that NULL was passed to memcpy, but that pbuf was NULL after a call to adjbuf. this should never happen but it did here because minlen was 0, (set to n instead of n + 1) which was corrected in todd's code. thanks miguel for catching the use-after-free with valgrind.