Audit string coercion throughout luv #397
Labels
Comments
|
Confirmed that the local uv = require('luv')
local path = "_test_"
local fd = assert(uv.fs_open(path, "w", tonumber("0666", 8)))
print("writing")
do
-- the number here gets coerced into a string
local t = {"with\n", 600, "lines\n"}
uv.fs_write(fd, t, -1, function()
print("written")
uv.fs_close(fd)
end)
end
local count = collectgarbage("count")
collectgarbage("collect")
print("garbage collected", count - collectgarbage("count"))
uv.run()Note that it's the coerced number that is failing; I couldn't get a crash when using strings (probably due to how strings are handled internally by Lua). When I run the above with PUC Lua 5.1 and Valgrind I get: |
squeek502
added a commit
to squeek502/luv
that referenced
this issue
Oct 28, 2019
When a number is coerced to a string, that string is temporary and could be garbage collected before the uv_buf_t is used, leading to use-after-free. Part of luvit#397
squeek502
added a commit
to squeek502/luv
that referenced
this issue
Oct 31, 2019
Part of luvit#397 luv_req_t now stores either one or multiple refs to the strings passed to functions that use uv_buf_t. This fixes those strings possibly being garbage collected between the time the uv_buf_t is constructed and the time the send actually completes. See luvit#397 for a more complete explanation of the problem. - Introduces a constant called LUV_REQ_MULTIREF which is an arbitrary negative value that luaL_ref will never return that signifies that luv_req_t.data is a LUA_NOREF-terminated array of `int`s and handles that case accordingly in luv_cleanup_req. This might be more hacky than it needs to be but it makes it possible to leave the structure of luv_req_t unchanged. - Maintains backwards compatibility in that it will still coerce numbers to strings (see luvit#426) - Factored out and moved `uv_buf_t`-related functions to misc.c (matches where uv_buf_t is in Libuv: http://docs.libuv.org/en/v1.x/misc.html#c.uv_buf_t) + Has the side-benefit of bringing udp_send/udp_try_send in line with the other send/write functions in that it will now accept either a string or a table of strings (before the udp send functions only accepted a single string)
squeek502
added a commit
to squeek502/luv
that referenced
this issue
Oct 31, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
From #127 (comment)
This is something I ran into when writing #393 (if
lua_isstringwas the first if check, then a number passed as the argument would return true fromlua_isstringand get coerced into a string) and it's something that we need to be more careful of and are probably using incorrectly.The text was updated successfully, but these errors were encountered: