GothenburgBitFactory / taskwarrior

Taskwarrior - Command line Task Management
https://taskwarrior.org
MIT License
4.29k stars 285 forks source link

[TW-347] sync init crashes if client certification file is empty or invalid #382

Closed taskwarrior closed 6 years ago

taskwarrior commented 6 years ago

Marton Suranyi on 2014-01-30T17:28:07Z says:

Preconditions

Given task with version 2.3.0,

System info: Archlinux
3.12.9-1-ARCH #1 SMP PREEMPT Sun Jan 26 09:01:37 CET 2014 x86_64 GNU/Linux

GnuTLS: 3.2.9-1

Relevant taskrc:
taskd.certificate=\/home\/susu\/.task\/susu.cert.pem
taskd.key=\/home\/susu\/.task\/susu.key.pem
taskd.credentials=Public\/Susu\/hash
taskd.server=localhost:12345

susu.cert.pem is exists, but empty.

Action


When I execute 'task sync init', it crashes with segfault.

Result

Backtrace:
1. 0  0x00007ffff7b04c70 in ?? () from /usr/lib/libgnutls.so.28
1. 1  0x00007ffff7b11ee6 in gnutls_deinit () from /usr/lib/libgnutls.so.28
1. 2  0x000000000056959f in TLSClient::~TLSClient (this=0x7fffffffda60, 
    __in_chrg=)
    at /home/susu/build/task/src/task-2.3.0/src/TLSClient.cpp:115
1. 3  0x000000000054f5ca in CmdSync::send (this=0x7e2c10, to="localhost:12345", ca="", 
    certificate="/home/susu/.task/susu.cert.pem", key="/home/susu/.task/susu.key.pem", 
    trust=false, request=..., response=...)
    at /home/susu/build/task/src/task-2.3.0/src/commands/CmdSync.cpp:358
1. 4  0x000000000054d158 in CmdSync::execute (this=0x7e2c10, output="")
    at /home/susu/build/task/src/task-2.3.0/src/commands/CmdSync.cpp:170
1. 5  0x000000000046dfaa in Context::dispatch (this=0x7d7080 , out="")
    at /home/susu/build/task/src/task-2.3.0/src/Context.cpp:383
1. 6  0x000000000046d2db in Context::run (this=0x7d7080 )
    at /home/susu/build/task/src/task-2.3.0/src/Context.cpp:259
1. 7  0x000000000046b58a in main (argc=3, argv=0x7fffffffea28)
    at /home/susu/build/task/src/task-2.3.0/src/main.cpp:51

I've debugged the problem: gnutls_deinit is called with an invalid handle in TLSClient.cpp:115.
It is invalid, because it is not initialized to zero, and it is not yet gnutls_init()-ed.

An exception fired in TLSClient::init (line 183), that caused a destructor call with invalid address in _session

In my opinion it is not totally consistent that TLSClient::init() initializes a member which is free'd in the destructor (violates RAII).

However the trivial fix is attached.

taskwarrior commented 6 years ago

Migrated metadata:

Created: 2014-01-30T17:28:07Z
Modified: 2014-02-09T00:53:17Z
taskwarrior commented 6 years ago

Paul Beckingham on 2014-01-31T14:02:05Z says:

Thank you for the patch. Applied to both taskwarrior and taskserver.

pbeckingham commented 6 years ago

TW-347_crashfix.patch.txt