Account directive use-after-free vulnerability with deferred postings (TALOS-2017-0304, CVE-2017-2808)

[tbm]

Cory Duplantis and another member of Cisco Talos discovered and described an issue with ledger:

An exploitable use-after-free vulnerability exists in the account parsing component of the Ledger-CLI 3.1.1. A specially crafted ledger file can cause a use-after-free vulnerability resulting in arbitrary code execution. An attacker can convince a user to load a journal file to trigger this vulnerability.

CVE-2017-2808 was assigned.

[tbm]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff57d0560 in std::__detail::_List_node_base::_M_hook(std::__detail::_List_node_base*) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
(gdb) where
#0  0x00007ffff57d0560 in std::__detail::_List_node_base::_M_hook(std::__detail::_List_node_base*) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#1  0x00007ffff77973e1 in std::__cxx11::list<ledger::post_t*, std::allocator<ledger::post_t*> >::_M_insert<ledger::post_t* const&> (this=0x98, 
    __position=<error reading variable: Cannot access memory at address 0x98>, __args#0=@0x7fffffffae80: 0x5555559171d0)
    at /usr/include/c++/6/bits/stl_list.h:1771
#2  0x00007ffff7878834 in std::__cxx11::list<ledger::post_t*, std::allocator<ledger::post_t*> >::push_back (this=0x98, __x=@0x7fffffffae80: 0x5555559171d0)
    at /usr/include/c++/6/bits/stl_list.h:1098
#3  0x00007ffff78726ff in ledger::account_t::add_post (this=0x0, post=0x5555559171d0) at /home/tbm/tmp/src/ledger/src/account.cc:130
#4  0x00007ffff7872d65 in ledger::account_t::apply_deferred_posts (this=0x555555917400) at /home/tbm/tmp/src/ledger/src/account.cc:175
#5  0x00007ffff7872f16 in ledger::account_t::apply_deferred_posts (this=0x55555590b5e0) at /home/tbm/tmp/src/ledger/src/account.cc:182
#6  0x00007ffff784f95b in ledger::journal_t::read_textual (this=0x55555590b3b0, context_stack=...) at /home/tbm/tmp/src/ledger/src/textual.cc:2026
#7  0x00007ffff78696b8 in ledger::journal_t::read (this=0x55555590b3b0, context=...) at /home/tbm/tmp/src/ledger/src/journal.cc:505
#8  0x00007ffff78181e5 in ledger::session_t::read_data (this=0x55555590aae0, master_account="") at /home/tbm/tmp/src/ledger/src/session.cc:171
#9  0x00007ffff7818ab6 in ledger::session_t::read_journal_files (this=0x55555590aae0) at /home/tbm/tmp/src/ledger/src/session.cc:203
#10 0x0000555555659c98 in ledger::global_scope_t::execute_command (this=0x55555590a4f0, args=std::__cxx11::list = {...}, at_repl=false)
    at /home/tbm/tmp/src/ledger/src/global.cc:226
#11 0x000055555565a2fb in ledger::global_scope_t::execute_command_wrapper (this=0x55555590a4f0, args=std::__cxx11::list = {...}, at_repl=false)
    at /home/tbm/tmp/src/ledger/src/global.cc:271
#12 0x0000555555638642 in main (argc=4, argv=0x7fffffffe7d8, envp=0x7fffffffe800) at /home/tbm/tmp/src/ledger/src/main.cc:125
(gdb) 

[tbm]

Test case (slightly modified from Cory's):

2017/3/17 deferred posting
    <deferred posting>

[tbm]

The background on these deferred postings is here: https://groups.google.com/forum/#!topic/ledger-cli/ooxbPVRinSs

[mbudde]

I don't think the analysis in the CVE report is complete. To me it looks like the bug happens because the 'all null' transaction is discarded:

ledger/src/xact.cc

Lines 407 to 408 in 53f4035

	if (all_null)
	return false; // ignore this xact completely

But right before that, in the loop checking for not-null posts, the deferred post is added to a map on the account containing deferred postings. It's when this map is read in account_t::apply_deferred_posts() that the use-after-free happens because the all-null transaction has been freed.

ledger/src/xact.cc

Lines 398 to 399 in 53f4035

	if (post->has_flags(POST_DEFERRED))
	post->account->add_deferred_post(id(), post);

ledger/src/account.cc

Lines 155 to 168 in 53f4035

	void account_t::add_deferred_post(const string& uuid, post_t * post)
	{
	if (! deferred_posts)
	deferred_posts = deferred_posts_map_t();
	deferred_posts_map_t::iterator i = deferred_posts->find(uuid);
	if (i == deferred_posts->end()) {
	posts_list lst;
	lst.push_back(post);
	deferred_posts->insert(deferred_posts_map_t::value_type(uuid, lst));
	} else {
	(*i).second.push_back(post);
	}
	}

Could a fix be to change xact_base_t::finalize() to return true in case an all-null transaction contains a least one deferred post? I don't know if there are any bad side-effects from keeping an all-null transaction.