[tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call

[tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call

[Serge Petrenko]

Calling box.schema.user:passwd() instead of box.schema.user.passwd()
leads to lua passing box.schema.user table as first function argument.
This case was unhandled in box.schema.user.passwd() function and lead to
an assertion failure.
Fix this by handling the aforementioned case and add a test.

Closes #3659
---
https://github.com/tarantool/tarantool/issues/3659
https://github.com/tarantool/tarantool/tree/sp/gh-3659-assert-in-user-passwd

 src/box/lua/schema.lua | 2 +-
 test/box/misc.result   | 8 ++++++++
 test/box/misc.test.lua | 6 ++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index 540a2a5fd..6b0c1cbb7 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -2025,7 +2025,7 @@ local function chpasswd(uid, new_password)
 end
 
 box.schema.user.passwd = function(name, new_password)
-    if name == nil then
+    if name == nil or type(name) == 'table' then
         box.error(box.error.PROC_LUA, "Usage: box.schema.user.passwd([user,] password)")
     end
     if new_password == nil then
diff --git a/test/box/misc.result b/test/box/misc.result
index 62376754e..6f05df0fa 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1148,6 +1148,14 @@ s = box.schema.space.create('test', {user="no_such_user"})
 ---
 - error: User 'no_such_user' is not found
 ...
+--
+-- gh-3659 assertion failure on improper call to
+-- box.schema.user:passwd()
+--
+box.schema.user:passwd()
+---
+- error: 'Usage: box.schema.user.passwd([user,] password)'
+...
 -- Too long WAL write warning (gh-2743).
 s = box.schema.space.create('test')
 ---
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index d6815645e..79c74ecbb 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -320,6 +320,12 @@ s:drop()
 --
 s = box.schema.space.create('test', {user="no_such_user"})
 
+--
+-- gh-3659 assertion failure on improper call to
+-- box.schema.user:passwd()
+--
+box.schema.user:passwd()
+
 -- Too long WAL write warning (gh-2743).
 s = box.schema.space.create('test')
 _ = s:create_index('pk')
-- 
2.15.2 (Apple Git-101.1)

Re: [tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call

[Vladimir Davydov]

On Mon, Sep 10, 2018 at 04:36:43PM +0300, Serge Petrenko wrote:
> Calling box.schema.user:passwd() instead of box.schema.user.passwd()
> leads to lua passing box.schema.user table as first function argument.
> This case was unhandled in box.schema.user.passwd() function and lead to
> an assertion failure.
> Fix this by handling the aforementioned case and add a test.
> 
> Closes #3659
> ---
> https://github.com/tarantool/tarantool/issues/3659
> https://github.com/tarantool/tarantool/tree/sp/gh-3659-assert-in-user-passwd
> 
>  src/box/lua/schema.lua | 2 +-
>  test/box/misc.result   | 8 ++++++++
>  test/box/misc.test.lua | 6 ++++++
>  3 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
> index 540a2a5fd..6b0c1cbb7 100644
> --- a/src/box/lua/schema.lua
> +++ b/src/box/lua/schema.lua
> @@ -2025,7 +2025,7 @@ local function chpasswd(uid, new_password)
>  end
>  
>  box.schema.user.passwd = function(name, new_password)
> -    if name == nil then
> +    if name == nil or type(name) == 'table' then
>          box.error(box.error.PROC_LUA, "Usage: box.schema.user.passwd([user,] password)")
>      end
>      if new_password == nil then

This isn't a proper fix, because the following command still crashes:

  box.schema.user.passwd(123)

And this one crashes too:

  box.session.su('admin', function(x) return #x end, 123)

Making error messages user-friendly is good, but I think that #3659 is
about invalid usage of luaT_error in lbox_session_su...

Re: [tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call

[Serge Petrenko]

[-- Attachment #1: Type: text/plain, Size: 2570 bytes --]



> 14 сент. 2018 г., в 18:17, Vladimir Davydov <vdavydov.dev@gmail.com> написал(а):
> 
> On Mon, Sep 10, 2018 at 04:36:43PM +0300, Serge Petrenko wrote:
>> Calling box.schema.user:passwd() instead of box.schema.user.passwd()
>> 
> 
> This isn't a proper fix, because the following command still crashes:
> 
>  box.schema.user.passwd(123)
> 
> And this one crashes too:
> 
>  box.session.su('admin', function(x) return #x end, 123)
> 
> Making error messages user-friendly is good, but I think that #3659 is
> about invalid usage of luaT_error in lbox_session_su...

Hi! I found the cause of this and fixed the issue. I also removed the type check since it
seems to be outside of the patch scope now. New diff is below.

---
 src/box/lua/session.c  | 5 ++++-
 test/box/misc.result   | 9 +++++++++
 test/box/misc.test.lua | 6 ++++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index b2e1400b6..7541da0a7 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -201,8 +201,11 @@ lbox_session_su(struct lua_State *L)
 	/* Restore the original credentials. */
 	fiber_set_user(fiber(), old_credentials);
 
-	if (error)
+	if (error) {
+		luaT_toerror(L);
 		luaT_error(L);
+	}
+
 	return lua_gettop(L) - 1;
 }
 
diff --git a/test/box/misc.result b/test/box/misc.result
index 62376754e..4ee4797d0 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1148,6 +1148,15 @@ s = box.schema.space.create('test', {user="no_such_user"})
 ---
 - error: User 'no_such_user' is not found
 ...
+--
+-- gh-3659 assertion failure after an error in code called from
+-- box.session.su()
+--
+box.session.su("admin", function(x) return #x end, 3)
+---
+- error: '[string "return box.session.su("admin", function(x) re..."]:1: attempt to
+    get length of local ''x'' (a number value)'
+...
 -- Too long WAL write warning (gh-2743).
 s = box.schema.space.create('test')
 ---
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index d6815645e..ee81c7be1 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -320,6 +320,12 @@ s:drop()
 --
 s = box.schema.space.create('test', {user="no_such_user"})
 
+--
+-- gh-3659 assertion failure after an error in code called from
+-- box.session.su()
+--
+box.session.su("admin", function(x) return #x end, 3)
+
 -- Too long WAL write warning (gh-2743).
 s = box.schema.space.create('test')
 _ = s:create_index('pk')
-- 
2.15.2 (Apple Git-101.1)

[-- Attachment #2: Type: text/html, Size: 4729 bytes --]

Re: [tarantool-patches] [PATCH] lua: fix assertion failure after improper box.schema.user:passwd() call

[Vladimir Davydov]

On Mon, Sep 17, 2018 at 05:48:53PM +0300, Serge Petrenko wrote:
> diff --git a/src/box/lua/session.c b/src/box/lua/session.c
> index b2e1400b6..7541da0a7 100644
> --- a/src/box/lua/session.c
> +++ b/src/box/lua/session.c
> @@ -201,8 +201,11 @@ lbox_session_su(struct lua_State *L)
>  	/* Restore the original credentials. */
>  	fiber_set_user(fiber(), old_credentials);
>  
> -	if (error)
> +	if (error) {
> +		luaT_toerror(L);
>  		luaT_error(L);
> +	}
> +

Turns out we can simply use lua_error() instead.
I fixed it and pushed the patch to 1.10.