After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677741 - [PATCH] pan crashes while fetching new headers
[PATCH] pan crashes while fetching new headers
Status: RESOLVED FIXED
Product: Pan
Classification: Other
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: pan-maint
pan-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-09 01:00 UTC by Scott Tsai
Modified: 2012-06-09 15:28 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
pan-0.137-validate-pass-after-get-server-auth.patch (399 bytes, patch)
2012-06-09 01:00 UTC, Scott Tsai 		none Details | Review
DataImpl-password_decrypt-ensure-pw.pw-is-valid-on-r.patch (807 bytes, patch)
2012-06-09 14:59 UTC, Scott Tsai 		none Details | Review

Description Scott Tsai 2012-06-09 01:00:39 UTC 
Created attachment 216021 [details] [review]
pan-0.137-validate-pass-after-get-server-auth.patch

After upgrading to the pan-0.137 package in Fedora 17, pan started segfaulting on Groups -> "Get new headers in subscribed groups". The Fedora pan packages were always built with Gnome keyring enabled. A analysis of the crash, a backtrace and a patch against pan-0.137 is provided below. (I believe the same bug can happen on pan git master)

Analysis:
1. While trying to get new headers from news.gmane.org, pan::DataImpl::get_server_auth(server, user, pass) can set the out parameter "pass" to -1 on a build with Gnome keyring support enabled.

I've verified that this always in the "GNOME_KEYRING_RESULT_OK" case.

2. In NNTP_Pool::on_socket_created(), the "gchar *pass" with an invalid address of -1 would be passed to the pan:NNTP ctor, which causes a crash when it tries to initialize the "std::string _password" from the invalid pointer.

Backtrace:

+ Trace 230331

  • #0 __strlen_sse2_pminub
    at ../sysdeps/x86_64/multiarch/strlen-sse2-pminub.S line 50
  • #1 length
    at /usr/src/debug/gcc-4.7.0-20120507/obj-x86_64-redhat-linux/x86_64-redhat-linux/libstdc++-v3/include/bits/char_traits.h line 261
  • #3 pan::NNTP_Pool::on_socket_created
    at nntp-pool.cc line 187
  • #4 (anonymous namespace)::ThreadWorker::on_worker_done
    at socket-impl-main.cc line 113
  • #5 pan::WorkerPool::Worker::main_thread_cleanup
    at worker-pool.cc line 90
  • #6 pan::WorkerPool::Worker::main_thread_cleanup_cb
    at worker-pool.cc line 82
  • #7 g_main_dispatch
    at gmain.c line 2539
  • #8 g_main_context_dispatch
    at gmain.c line 3075
  • #9 g_main_context_iterate
    at gmain.c line 3146
  • #10 g_main_loop_run
    at gmain.c line 3340
  • #11 IA__gtk_main
    at gtkmain.c line 1256
  • #12 run_pan_in_window
    at pan.cc line 536
  • #13 main
    at pan.cc line 1034
  • #3 pan::NNTP_Pool::on_socket_created
    at nntp-pool.cc line 187 
$7 = (gchar *) 0xffffffffffffffff <Address 0xffffffffffffffff out of bounds>
Comment 1 Heinrich Müller 2012-06-09 12:13:36 UTC 
was already fixed in git, please open again if it doesn't work for you.
Comment 2 Scott Tsai 2012-06-09 14:58:27 UTC 
(In reply to comment #1)
Heinrich,
I checked with git master and the same bug can be triggered with these steps:

1. "Edit" -> "Edit Preferences" -> Miscellaneous Tab -> check "Store passwords in GNOME Keyring" at the bottom of the page.
2. "Groups"  -> "Get new headers in subscribed groups"

I've read the code some more, the problem is that in DataImpl::get_server_auth(),
the "pw" field of the "PasswordData pw" local variable can be used without initialization if GNOME the server doesn't require a password and GNOME keyring is enabled. I've updated my trivial patch to make DataImpl::password_decrypt() always set "pw.pw" to a valid string.
Comment 3 Scott Tsai 2012-06-09 14:59:34 UTC 
Created attachment 216033 [details] [review]
DataImpl-password_decrypt-ensure-pw.pw-is-valid-on-r.patch
Comment 4 Heinrich Müller 2012-06-09 15:28:11 UTC 
Ok, done, thanks.