GNOME Bugzilla – Bug 683118
gvfs-sftpd crashes in g_utf8_validate() if the target host is not in .ssh/known_hosts
Last modified: 2012-09-04 16:46:44 UTC
Originally reported in https://launchpad.net/bugs/1033275: When trying to connect to an ssh server with nautilus or "gvfs-mount sftp://...", gvfs-sftpd crashes if the target host is not in ~/.ssh/known_hosts. Trace:
+ Trace 230771
This is the crux of the problem: in g_mount_source_ask_question() we get a valid char** choices and n_choices == 2: (gdb) p choices[0] $13 = 0x423a5c "Log In Anyway" (gdb) p choices[1] $14 = 0x423a6a "Cancel Login" (gdb) p choices[2] $15 = 0x6874756120656854 <Address 0x6874756120656854 out of bounds> (gdb) p n_choices $16 = 2 Then this gets passed to g_mount_source_ask_question_async(), which however entirely ignores n_choices and just passes choices on to gvfs_dbus_mount_operation_call_ask_question() in the autogenerated common/gvfsdbus.c. This calls g_variant_new(), which expects a NULL terminated array. With the simple g_variant_new() API we cannot actually pass an array length (unlike with g_variant_new_strv()), so I think gvfs should just ensure that choices is NULL-terminated. I'll work on a patch for this.
Yes, came across this two days ago, preparing a patch, give me few minutes. But yes, you got the point, we need NULL-terminated arrays for the gdbus calls.
Created attachment 223079 [details] [review] sftp: Fix crash if the target host is not in .ssh/known_hosts This is the simplest possible patch, and it fixes the problem. It's not terribly elegant, though. A better approach might be to drop the n_choices argument from all the internal API completely, and just demand NULL termination.
Created attachment 223080 [details] [review] gmountsource: Always use NULL-terminated arrays (In reply to comment #3) > It's not terribly elegant, though. A better approach might be to drop the > n_choices argument from all the internal API completely, and just demand NULL > termination. Thanks for the patch, breaking internal API was my intention actually. We don't transfer the n_choices argument over d-bus anyway.
Bug 682496 is related issue.
Thanks Tomas. I had a similar patch, but currently struggling with a build failure as gvfs master doesn't build against glib 2.33.10. I'm just building in jhbuild, but seems you beat me to it anyway now. :-) Sorry for the mid-air collision, and thanks for the super-fast response!
For the record, I reproduced this bug in a test case in the integration test suite that I'm developing: http://bazaar.launchpad.net/~ubuntu-branches/ubuntu/quantal/gvfs/quantal/revision/232 (see https://mail.gnome.org/archives/gvfs-list/2012-August/msg00000.html for the beginnings of dicussions in which form to get that upstream). test_rsa (__main__.Sftp) sftp://localhost with RSA authentication ... ok test_unknown_host (__main__.Sftp) sftp:// with RSA authentication for unknown host ... FAIL ====================================================================== FAIL: test_unknown_host (__main__.Sftp) sftp:// with RSA authentication for unknown host ---------------------------------------------------------------------- Traceback (most recent call last):
+ Trace 230772
self.assertTrue('Login dialog cancelled' in err, err)
---------------------------------------------------------------------- Ran 2 tests in 3.073s With the fix, the test case works. I also verified it manually by temporarily moving away my ~/.ssh/known_hosts file.
*** Bug 683334 has been marked as a duplicate of this bug. ***
Oh, nice catch with the test suite. I'll follow up in the list in next few days, trying to get more QA people involved. It certainly looks good! I've pushed the patch in comment 4 as commit 085c2885ff7f1acaf7a5238f85732d361ad55fc9. Thanks for confirming it works.