From 6b46ea5706556c9fb400ac87b1064d8263d549fa Mon Sep 17 00:00:00 2001 From: Alexandre Erwin Ittner Date: Thu, 18 Nov 2010 16:03:08 -0200 Subject: [PATCH] Prevent crashes with double-freed conversion descriptors It was possible to crash the interpreter by forcefully calling the __gc metamethod twice. This commit fixes the bug. --- README | 4 ++++ luaiconv.c | 19 +++++++++++++++---- test_iconv.lua | 11 +++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/README b/README index 6d63a74..2990caa 100644 --- a/README +++ b/README @@ -88,6 +88,10 @@ directory from package.cpath). iconv.ERROR_INCOMPLETE An incomplete character was found in the input sequence. + iconv.ERROR_FINALIZED + Trying to use an already-finalized converter. This usually means + that the user was tweaking the garbage collector private methods. + iconv.ERROR_UNKNOWN There was an unknown error. diff --git a/luaiconv.c b/luaiconv.c index 2130000..4122f2d 100644 --- a/luaiconv.c +++ b/luaiconv.c @@ -65,6 +65,7 @@ #define ERROR_INVALID 2 #define ERROR_INCOMPLETE 3 #define ERROR_UNKNOWN 4 +#define ERROR_FINALIZED 5 static void push_iconv_t(lua_State *L, iconv_t cd) { @@ -77,9 +78,7 @@ static void push_iconv_t(lua_State *L, iconv_t cd) { static iconv_t get_iconv_t(lua_State *L, int i) { if (luaL_checkudata(L, i, ICONV_TYPENAME) != NULL) { iconv_t cd = UNBOXPTR(L, i); - if (cd == (iconv_t) NULL) - luaL_error(L, "attempt to use an invalid " ICONV_TYPENAME); - return cd; + return cd; /* May be NULL. This must be checked by the caller. */ } luaL_argerror(L, i, lua_pushfstring(L, ICONV_TYPENAME " expected, got %s", luaL_typename(L, i))); @@ -110,6 +109,12 @@ static int Liconv(lua_State *L) { size_t ret = -1; int hasone = 0; + if (cd == NULL) { + lua_pushstring(L, ""); + lua_pushnumber(L, ERROR_FINALIZED); + return 2; + } + outbuf = (char*) malloc(obsize * sizeof(char)); if (outbuf == NULL) { lua_pushstring(L, ""); @@ -192,8 +197,13 @@ static int Liconvlist(lua_State *L) { static int Liconv_close(lua_State *L) { iconv_t cd = get_iconv_t(L, 1); - if (iconv_close(cd) == 0) + if (cd != NULL && iconv_close(cd) == 0) { + /* Mark the pointer as freed, preventing interpreter crashes + if the user forces __gc to be called twice. */ + void **ptr = lua_touserdata(L, 1); + *ptr = NULL; lua_pushboolean(L, 1); /* ok */ + } else lua_pushnil(L); /* erro */ return 1; @@ -223,6 +233,7 @@ int luaopen_iconv(lua_State *L) { TBL_SET_INT_CONST(L, "ERROR_NO_MEMORY", ERROR_NO_MEMORY); TBL_SET_INT_CONST(L, "ERROR_INVALID", ERROR_INVALID); TBL_SET_INT_CONST(L, "ERROR_INCOMPLETE", ERROR_INCOMPLETE); + TBL_SET_INT_CONST(L, "ERROR_FINALIZED", ERROR_FINALIZED); TBL_SET_INT_CONST(L, "ERROR_UNKNOWN", ERROR_UNKNOWN); lua_pushliteral(L, "VERSION"); diff --git a/test_iconv.lua b/test_iconv.lua index 0cc9e3a..ede8a04 100644 --- a/test_iconv.lua +++ b/test_iconv.lua @@ -97,3 +97,14 @@ check_one(termcs, "utf8", utf8) check_one(termcs, "utf16", utf16) check_one(termcs, "EBCDIC-CP-ES", ebcdic) + +-- The library must never crash the interpreter, even if the user tweaks +-- with the garbage collector methods. +local cd = iconv.new("iso-8859-1", "utf-8") +local _, e = cd:iconv("atenção") +assert(e == nil, "Unexpected conversion error") +local gc = getmetatable(cd).__gc +gc(cd) +local _, e = cd:iconv("atenção") +assert(e == iconv.ERROR_FINALIZED, "Failed to detect double-freed objects") +gc(cd)