lunarmodules / luasql

LuaSQL is a simple interface from Lua to a DBMS.
http://lunarmodules.github.io/luasql
547 stars 192 forks source link

luasql.postgres: SIGSEGV and crash if conn:escape(string) and returned string >8199 bytes #55

Open shripchenko opened 8 years ago

shripchenko commented 8 years ago

This sample script

local luasql = require "luasql.postgres"
local env = assert(luasql.postgres())
local conn = assert(env:connect(<your conn string here>))

for i=4080,5000,1 do
        local s1 = string.rep("'", i)
        local s2 = conn:escape(s1)
        print(string.format("s1=%i, s2=%i", string.len(s1), string.len(s2)))
end

will be failed after s1=4099, s2=8198 iteration and generate either SIGSEGV, or random error like:

s1=4099, s2=8198
Segmentation fault
s1=4099, s2=8198
lua: bad argument #1 to 'string.len' (string expected, got function)
stack traceback:
        [C]: in function 'string.len'
        [C]: in function 'len'
        ./bla2.lua:8: in main chunk
        [C]: in ?

some insights after brief looking to code: luaL_Buffer b; - default buffer size of 8192 char *to = luaL_buffinitsize (L, &b, 2*len+1); - not checking actual returned buffer size luaL_buffinit (L, &b); - init buffer with default size of 8192 regardles of len len = PQescapeStringConn (conn->pg_conn, to, from, len, &error); - len parameter does not take into account an actual buffer size

Environment:

root@srv1:/opt/bla/scripts# lua -v
Lua 5.2.3  Copyright (C) 1994-2013 Lua.org, PUC-Rio

root@srv1:/opt/bla/scripts# uname -a
Linux srv1 3.16.0-4-amd64 #1 SMP Debian 3.16.7-ckt20-1+deb8u3 (2016-01-17) x86_64 GNU/Linux

root@srv1:/opt/bla/scripts# luarocks show luasql-postgres

LuaSQL-Postgres 2.3.3-1 - Database connectivity for Lua (Postgres driver)

LuaSQL is a simple interface from Lua to a DBMS. It enables a Lua program to
connect to databases, execute arbitrary SQL statements and retrieve results in
a row-by-row cursor fashion.

License:        MIT/X11
Homepage:       http://www.keplerproject.org/luasql/
Installed in:   /usr/local

Modules:
        luasql.postgres (/usr/local/lib/lua/5.2/luasql/postgres.so)
joell commented 8 years ago

I've run into the same issue and discovered it is alleviated by downgrading to version 2.3.0. It looks like the changes in 2.3.1 introducing the segfault-related code.

mpeterv commented 8 years ago

The issue is that fallback implementation of luaL_buffinitsize calls luaL_prepbuffer instead of luaL_prepbuffsize. luaL_prepbuffer is equivalent to luaL_prepbuffsize with buffer size LUAL_BUFFERSIZE (8192). Incidentally, feature detection is invalid (luaL_buffinitsize is not a macro), which makes this bug appear even for Lua 5.2 and 5.3.

mpeterv commented 8 years ago

Fixed on master branch.

joell commented 8 years ago

@mpeterv: Please be aware that your use of luaL_prepbuffsize in 6316be8d0fa241657b4d337f29c4b740452c2dc9 breaks backward compatibility with Lua 5.1. Up until this point LuaSQL, including 2.3.3, worked fine with Lua 5.1. I'm not sure if a decision to deprecate 5.1 support was made after the 2.3.3 release, but I thought you'd like be aware.

For what it's worth, my use case for LuaSQL was as a dependency to a Mozilla Heka output plugin, and the latest release of Heka (0.10.0) still relies on Lua 5.1 for its sandboxes, so for now I'll have to continue using LuaSQL 2.3.0 to work around the problem.

mpeterv commented 8 years ago

@joell, sorry, that was unintentional.

@tomasguisasola if malloc can't be avoided while maintaining correctness @4acc737 should be (partially) reverted, right?

hishamhm commented 7 years ago

couldn't one just use lua_newuserdata instead of malloc to allocate the memory that's temporarily needed there and just leave it to the Lua GC to collect it?

tomasguisasola commented 7 years ago

lua_newuserdata is not the correct choice. The lua string bufer is (http://www.lua.org/manual/5.3/manual.html#luaL_Buffer).

I think now it is correctly used and it is working with Lua 5.1, 5.2 and 5.3. I couldn't check it with Lua 5.0, but will try... I would be glad if both joell and mpeterv could try this new version: 2.3.4. Let me know whether it is working!