5.1. Libbitcoin APIs: Evaluation
The Libbitcoin libraries are available on GitHub (
https://github.com/Libbitcoin/ accessed 03.03.2020). The developer’s documentation can be found on Libbitcoin’s Wiki page. This evaluation is based on those Libbitcoin Git repositories. Running Tokei (
https://github.com/Aaronepower/tokei accessed 03.03.2020) on those repositories results in a total of 1393 C++ source and header files and 169689 physical lines of C++ source code. The 88679 of these physical C++ lines of source code belong to the foundation Libbitcoin library, which was renamed to Libbitcoin-system shortly after this evaluation had been concluded. It is worth mentioning that some of the below evaluation uses the Cognitive Walkthrough methodology of Callaghan (2010) in [
30], where the authors of the paper “walk through” the Libbitcoin’s code line by line to gain an understanding of the library concerning the heuristic under investigation.
KCE-1 Names should be self-explanatory
Using the Cognitive Walkthrough methodology shows that Libbitcoin’s API uses expressive function names that, mostly, avoid the use of abbreviations. While the use of abbreviations such as multisig (i.e., Multisignature) is domain terminology and, therefore, is sensible to use, the use of min_version and max_version in or maxmoney in is slightly inconsistent compared to the ‘minimum’ and ‘maximum’ used in other parts of the API.
KCE-2 Data types should be as specific as possible to make the code more readable
The choice of data types is exemplary in Libbitcoin. The object model is well defined, with dedicated types for each entity. The numeric data types used have a predefined bit width. The standard C++ data types and classes are used appropriately. However, the boolean flags are used plentifully throughout the API (see Listing 1). The meanings of the boolean flags are difficult to remember, making it necessary to regularly look up their purpose in the documentation. This is a usability problem.
Listing 1.check-cpp-api output for KCE-2 heuristic (truncated). |
> find Libbitcoin-all -name ’*.hpp’ -exec check-cpp-api -kc-1-1-case-type=snake {} + | grep KCE-2 .../Bitcoin/chain/block.hpp:93: KCE-2-2: boolean parameter .../Bitcoin/chain/header.hpp:109: KCE-2-2: boolean parameter .../Bitcoin/chain/input.hpp:90: KCE-2-2: boolean parameter ...
|
To investigate this usability problem further, Listing 2 shows one of the Libbitcoin’s functions that uses a boolean flag. As one can see in Listing 3, the same function could be made more expressive using an enumeration. Finally, Listing 4 contrasts the usage of these two functions illustrating the usability advantage of using enumerations for flags, compared to boolean values, as suggested by our tools.
Listing 2. Boolean flag used in parse_signature in math/elliptic_curve.hpp. |
|
Listing 3. Hypothetical parse_signature using an enumeration. |
enum class der_enforcement { lax, strict }; BC_API bool parse_signature( ec_signature& out, const der_signature& der_signature, der_enforcement der_enforcement);
|
Listing 4. Using boolean vs. enumeration for flags. |
// true: lax or strict? parse_signature(ec_sig, der_sig, true); parse_signature(ec_sig, der_sig, der_enforcement::strict);
|
KCS-2 When reading code that uses the API, it should be easy to understand what that code does
We observe that although Libbitcoin does not support the chaining of methods, this does not adversely affect the readability of Libbitcoin’s code. Libbitcoin code is easy to read because its types, methods and functions are meaningfully and consistently named. In Libbitcoin’s codebase, there is a slight majority of positive over negative conditionals. Furthermore, Libbitcoin seems to adhere to C++ standard conventions as much as possible, which makes its code easy to read for anyone who is familiar with modern C++ code.
KCF-2 Functions should perform only the tasks described in their names
One of the guidelines in KCF-2 suggests that methods should not have side effects. One instance of side effects in C++ is methods that change the parameterised object(s) [
152]. C++ offers the
const qualifier to tell both the compiler and the users of an API that particular methods are free of side effects (i.e., that their execution will not change the observable state of their related objects). Libbitcoin uses
const meticulously in method declarations. All property accessors and predicate methods (i.e., methods starting with
is_ and
has_) are
const-qualified. The same is true for other methods whose names indicate that they should not influence an object’s state.
KC-1 The API should be consistent with itself
Libbitcoin’s naming is mostly consistent. The snake case naming convention is consistently used. Parameters seem to be consistently ordered. Out-parameters typically appear before in-parameters in function declarations throughout Libbitcoin. In the rare instances where this is not the case, such as with some of the network_address methods in , the deviation is consistent. However, a few API inconsistencies do exist. Errors, for example, are not handled consistently throughout the whole codebase. Some functions use a boolean return value to indicate success/failure of a function, as shown in Listing 5, while others return a failure code, as seen in Listing 6, and yet others might throw an exception, as seen in Listing 7.
Listing 5.decode_base58 in f ormats/base_58.hpp. |
|
Listing 6. Some of block’s methods return an error code in chain/block.hpp. |
code accept_transactions( const chain_state& state) const; code connect() const; code connect( const chain_state& state) const; code connect_transactions( const chain_state& state) const;
|
Listing 7.to_utf8 throws an exception in unicode/unicode.cpp. |
size_t to_utf8(char out[], size_t out_bytes, const wchar_t in[], size_t in_chars) { ... if (bytes > out_bytes) throw std::ios_base::failure( ‘‘utf8 buffer is too small’’); return bytes; }
|
Additionally, some objects can be in an invalid state. While most of these objects provide an is_valid method (e.g., chain::header in ), others overload operator bool (e.g., wallet::stealthaddress in ).
KC-2 The API should be consistent with standard conventions
The standard C++ language does not prescribe any standard for formatting code or naming entities. However, some naming and formatting standards have been established. In any case, a good recommendation is to be consistent within a codebase with a specific standard that has been chosen [
153]. Libbitcoin mostly adheres to this recommendation. Property accessors mostly use the
property/set_property naming convention. In-parameters are passed by value for simple types, constant reference for complex types, or rvalue-reference for consumed parameters. The out-parameters are passed by reference. Non-static property accessors for querying property values (i.e., getters) and predicates (i.e.,
is_) are properly qualified with the
const keyword in most cases. While a few minor inconsistencies do exist, they look like oversights rather than deliberate deviations. The
BC_PROPERTY_GET_REF macro defined in
results in property getters with a
get_ prefix (instead of no prefix) and without the
const qualifier. The
BC_PROPERTY macro defined in
also results in property getters with a
get_ prefix. However, this macro does apply the
const qualifier to the resulting property accessors. Further examples are the use of pass by constant value in the
header_organizer constructor in
or the pass by value instead of pass by constant reference of
data_slice in most of the APIs.
KM-1 The API should be easy to remember
Mosqueira-Rey et al. (2018) in [
29] suggest that methods should not have long names. However, neither Mosqueira-Rey et al. (2018) nor the sources they reference for that guideline provide a concrete value of a sensible maximum length for method names. The work of Scheller and Kühn in [
153] on page 15, which Mosqueira-Rey et al. (2018) reference for that guideline, suggests that “the length of names does not have a significant impact on usability”. The example name that was given by Mosqueira-Rey et al. (2018) for that guideline seems difficult, long and does not make much sense. The longest methods in Libbitcoin’s public API are
is_pay_witness_script_hash_pattern in
,
transaction_pool_fetch_transaction,
transaction_pool_fetch_transaction2,
blockchain_fetch_transaction_index, and
block-chain_fetch_unspent_outputs in
. The names of those functions are expressive and consistent with the names of other, similar functions, and should, therefore, be easy to remember despite their length.
It should be easier to remember how to use functions with fewer parameters compared to ones that have many parameters. The suggestion is to aim for four parameters or fewer, which seems a reasonable number to aim for but not a real hard limit [
29]. This study assumes at most six parameters to be a good compromise between usability and practicability. The method with the most parameters in Libbitcoin is the
version constructor in
, which requires nine arguments. There are two functions in Libbitcoin’s public API. with eight parameters each, namely
check_signature and
create_endorsement in
, both of which have eight parameters but require only six (i.e., the last two are optional parameters with default arguments). Three functions take seven arguments, including
get_map in
, the
program constructor in
,
create_key_pair in
. The
program constructor accepts one optional argument and
create_key_pair is commented to be deprecated.
Confusing arguments of the same data type can be particularly unsafe. Normally, compilers cannot detect such mistakes. Preconditions could be used as a countermeasure provided that the values of the concerned parameters do not overlap. What is more likely, however, is that arguments that are passed in the wrong order manifest as run-time errors. In the worst case, mixed up arguments having the same data type exhibit no errors. Instead, they make the application behave in a correct but undesired way. This study, therefore, investigates whether Libbitcoin had any APIs accepting more than three consecutive arguments with the same data type. Libbitcoin has only a single such function in its public API, namely the block_database constructor in , which has four consecutive parameters of type path, followed by two parameters of type size_t. Listing 8 shows the output of check-cpp-api that was used for the above analysis.
Listing 8.check-cpp-api output for the KM-1 heuristic (truncated). |
> find Libbitcoin-all -name ’*.hpp’ -exec check-cpp-api -kc-1-1-case-type=snake {} + | grep KM-1 .../Bitcoin/chain/chain_state.hpp:130: KM-1-2: too many parameters .../Bitcoin/chain/chain_state.hpp:223: KM-1-3: too many cons. params of same type .../Bitcoin/chain/script.hpp:136: KM-1-2: too many parameters .../Bitcoin/chain/script.hpp:176: KM-1-1: long function name ...
|
KM-2 The API should follow the terminology of the field
Although Libbitcoin mostly adheres to domain terminology, a few deviations do exist. The
header class in
exhibits the
Merkle property. However, the related domain terminology is Merkle Root, as can be seen in
Figure 1. Naming the property
merkle_root would, therefore, be more in line with Bitcoin terminology. Indeed, the
block class in
does exhibit the two-member functions
generate_merkle_root and
is_valid_merkle_root, which highlights the inconsistency.
KHS-1 Every element of the API should be documented
The Libbitcoin’s source code does contain some documentation. However, that documentation cannot be considered usable API reference documentation. While some documenting comments describe the purpose of the API method, most such comments are merely brief. Furthermore, some documenting comments seem contradictory, such as the one in Listing 9. If the terminology “is set” is appropriate, then why not call the function is_set or is_fork_set?
Listing 9.chain_state::is_enabled in chain/chain_state.hpp. |
|
Although most of Libbitcoin’s source code does not use structured comment blocks usable for generating reference documentation, some of it does. It is, however, neither complete nor up-to-date. An example is shown in Listing 10. Only one of the function’s three parameters is documented. Furthermore, the parameter name in the documentation does not match the function’s actual parameter name. Additionally, although the signature is an out-parameter, it is marked ’in’ in the structured documentation comment.
Listing 10.sign_message in wallet/message.hpp. |
/** * Signs a message using deterministic * signature. * @param[in] out_signature The * elements of in Bitcoin’s own * format. This should be base64 * encoded for presentation to the user. * @return true if wif is valid and * signature encoding is successful. */ BC_API bool sign_message( message_signature& signature, data_slice message, const ec_private& secret);
|
Some API documentation has misleading parameter annotations. The
@param[in] annotation in Listing 11 implies that the
list parameter is an input parameter into the function (
http://www.doxygen.nl/manual/commands.html accessed 03.03.2020). However, the fact that it is passed by reference, instead of by constant reference, indicates that the function might serve as both an input and an output parameter.
Listing 11.distinct in utility/collection.hpp. |
/** * Obtain the sorted distinct * elements of the list. * @param <Element> The list element type. * @param[in] list The list. * @return The sorted list reduced to * its distinct elements. */ template <typename Element> std::vector<Element>& distinct( std::vector<Element>& list);
|
Similarly, the key parameter of find_pair_position function declared in is annotated as an input parameter, as seen in Listing 12, but passed by reference in the function declaration. However, the function’s implementation in accepts the key parameter by constant reference, as Listing 13 shows. The reason why this works is that the function in Listing 13, whose definition is included in using an include statement, overloads the function with the same name in Listing 12.
Listing 12.find_pair_position declaration in utility/collection.hpp. |
/** * Find the position of a pair in an * ordered list. * @param <Pair> The type of list * member elements * @param[in] list The list to search. * @param[in] key The key to the element * to find. * @return The position or -1 if not found. */ template <typename Pair, typename Key> int find_pair_position( const std::vector<const Pair>& list, Key& key);
|
Listing 13.find_pair_position definition in utility/collection.hpp. |
template <typename Pair, typename Key> int find_pair_position( const std::vector<Pair>& list, const Key& key) { const auto predicate = [&] (const Pair& pair) { return pair.first == key; }; auto it = std::find_if( list.begin(), list.end(), predicate); if (it == list.end()) return -1; // Unsafe for use with lists greater // than max_int32 in size. Bitcoin_ASSERT(list.size() <= max_int32); return static_cast<int>( distance(list.begin(), it)); }
|
We conclude that without meaningful documentation that makes it easy on developers to use the APIs, it is hard to maintain the security of the code. It is worth noting that the learning resources of security have been studied in recent years by the academic community of usable security.
KHS-3 The API should properly identify deprecated classes and methods
Libbitcoin does identify some API functions in and some methods of the transaction class in as deprecated. Although Libbitcoin defines the BC_DEPRECATED macro in that would produce a compiler warning when a deprecated function is used, that macro is not used anywhere.
KHS-4 The API should supply helpful error information and, if possible, suggest a solution
Errors can be handled using various strategies in C++. Returning an error code and throwing exceptions are two such strategies. The Libbitcoin Wiki provides no treatment of how errors are handled within the library or how to deal with them when using the library. However, inspection of Libbitcoin’s APIs and implementation shows that Libbitcoin handles errors in three different ways. Some functions throw exceptions, some return a boolean value to indicate success or error and some return an error code. An advantage of exceptions over boolean or integer return values is that exception types already provide some information about the causes of errors/exceptions. Exceptions also make it more difficult to ignore erroneous events. However, Libbitcoin makes only little use of exceptions. Instead, it primarily relies on the returning of error codes and boolean error indications.
KHS-5 The API documentation should include code samples for the most common scenarios
Although some of Libbitcoin’s libraries contain an example directory, these directories contain only a few short examples. Libbitcoin’s Wiki provides 14 web pages with developer documentation on various topics. Additionally, 11 web pages with code examples related to the developer documentation topics are available on the Wiki. However, some of these examples use API functions that have been marked deprecated. One such example is the use of pseudo_random_fill in the example.
RU-1 The API should allow detecting and managing errors without breaking the execution or leaving the error undetected
Until C++ gets proper support for contract programming [
154], the verification of pre-conditions, post-conditions and invariants are sometimes checked using the assert macro. Software developers also often use assertions to verify assumptions [
155]. Libbitcoin seems to be using assertions for all these cases in various parts of the library. However, it does not do that consistently. Furthermore, in some places, assertions seem to be used inappropriately. Because assertions are not in effect in production builds, assertions should not be used to test conditions that may just as well happen in production. Listing 14 shows an assert that halts a non-production version of an application using Libbitcoin in case transaction store corruption is detected. However, the assumption is that the same condition might occur in production builds, which would lead to inconsistent results being returned.
Listing 14. Testing store corruption but only in non-production builds in inter f ace/block_chain.cpp. |
block_const_ptr block_chain::get_block( size_t height, bool witness, bool candidate) const { ... const auto result = database_.blocks().get( height, candidate); ... transaction::list txs; ... // False implies store corruption. DEBUG_ONLY(const auto value =) get_transactions( txs, result, witness); Bitcoin_ASSERT(value); // Use non-const header copy to // obtain move construction for txs. auto header = result.header(); return std::make_shared<const block> (std::move(header),std::move(txs)); }
|
A questionable use of assert is to check pointer variables before they are dereferenced, as can be found in Libbitcoin in a small number of cases. Listing 15 shows one such example.
Listing 15. Dereferencing a pointer variable that might be a nullptr in pools/header_entry.cpp. |
// Not callable if the entry is // a search key const hash_digest& header_entry:: parent() const { Bitcoin_ASSERT(header_); return header_->previous_block_hash(); }
|
Because Libbitcoin libraries are programmed in C++, the use of most data types will be checked at compilation time rather than at run-time. Libbitcoin also uses
enum class in many but not all cases.
enum class was introduced in C++ 11 (
http://www.stroustrup.com/C++11FAQ.html#enum accessed 03.03.2020) to improve the type safety of enumerated types.
RU-2 The API should facilitate managing non-common but correct situations without generating exceptions or forcing users to catch them
Although Listing 16 might suggest that there is potential for using optional return values in Libbitcoin, most of them would not be particularly beneficial. There are only a few cases in the Libbitcoin APIs where an optional value could be used instead of using the combination of an out-parameter and a boolean return value indicating success or failure. However, all of them are used to handle exceptional situations where continuing on the standard program path makes no sense. While the use of std::optional may offer some slight advantage over a combination of the boolean return value and out-parameter, such as, for example, the possibility to make the returned value constant, it does not simplify the client code significantly.
Listing 16.check-cpp-api output for the RU-2 heuristic (truncated). |
> find Libbitcoin-all -name ’*.hpp’ -exec check-cpp-api -kc-1-1-case-type=snake {} + | grep RU-2 .../Bitcoin/chain/block.hpp:98: RU-2: omission to use optional? .../Bitcoin/chain/block.hpp:99: RU-2: omission to use optional? .../Bitcoin/chain/compact.hpp:51: RU-2: omission to use optional? .../Bitcoin/chain/header.hpp:113: RU-2: omission to use optional? ...
|
RU-3 The API should not expose vulnerabilities that would allow users to make errors
While Libbitcoin seems to handle element accessibility purposefully, there are some class members whose accessibility could be further restricted. Various classes have protected methods, e.g., operation in or chain_state in , although they do not seem to be intended to serve as base classes. Sometimes compiler bugs mandate certain members to be protected although they could be private from a language point of view. However, such cases should visibly be documented in the code. Another case where accessibility should be further restricted is the BC_PROPERTY_GET_REF macro shown in Listing 17. The macro generates property accessors that return non-const references to private member variables, defeating the purpose of making these member variables private in the first place. The BC_PROPERTY_GET_REF macro, however, is only used in a single non-critical class of Libbitcoin.
Listing 17.BC_PROPERTY_GET_REF exposing private member by non-const reference in config/printer.hpp. |
|
Immutability is addressed nicely in Libbitcoin’s API. For example, those methods that do not need to change their related object, such as property getters, are properly qualified with the const keyword. However, immutability could be further promoted in client code by using optional return values instead of out-parameters and boolean return values. With a boolean return value and an out-parameter, a mutable temporary variable must be created and passed into the function as Listing 18 shows. Listing 19 demonstrates how the temporary variable could be made immutable with the help of an optional return value. By doing this, an added benefit would be allowing the use of type inference which further enhances the usability of the API.
Listing 18.create_ephemeral_key returning boolean success or failure value in math/stealth.hpp. |
BC_API bool create_ephemeral_key( ec_secret& out_secret, const data_chunk& seed); ... ec_secret ephemeral_private; if (create_ephemeral_key( ephemeral_private, seed)) initialize(ephemeral_private, address, seed, filter);
|
Listing 19. Hypothetical create_ephemeral_key alternative returning an optional value in math/stealth.hpp. |
BC_API std::optional<ec_secret> create_ephemeral_key( const data_chunk& seed); ... if (const auto ephemeral_private = create_ephemeral_key(seed)) initialize(*ephemeral_private, address, seed, filter);
|
There are no obvious threading issues in Libbitcoin from a API usability point of view. There are also no APIs in Libbitcoin returning allocated memory that must be manually freed by a user of the API. Furthermore, neither
cppcheck (
http://cppcheck.sourceforge.net/ accessed 03.03.2020) nor Clang Static Analyzer (
https://clang-analyzer.llvm.org/ accessed 03.03.2020) reported any memory-related issues.
SUC-1 The API should not compromise the confidentiality of the users’ personal information
Libbitcoin does not seem to acquire, store or process any personal information beyond what is necessary to fulfil its intended purpose. The sole information that could be considered personal is the one related to the wallet functionality which makes sense to use. All other data is related to the public blockchain.
SUA-1 The API should not compromise the security of the users’ assets
Not compromising the security of user assets should be a primary goal of any Bitcoin library. However, assessing whether an API constitutes a risk to user assets seems challenging when evaluating the API alone, not along with its implementation. For security-critical algorithms, including the ones related to AES, ECDSA, RIPEMD, SHA and scrypt, Libbitcoin uses external implementations. Libbitcoin provides unit tests for a lot of its code. However, the test coverage should be increased to include more error and corner cases.
5.2. Open-Source Projects: Libbitcoin APIs Evaluation
It is worth emphasising that the versification of how secure Libbitcoin’s APIs are is outside the scope of this research. Having evaluated Libbitcoin’s APIs in the previous sections, it is now time to investigate how such APIs are used in production. This subsection demonstrates the results of investigating the utilisation of Libbitcoin in the following open-source projects concerning the findings described in the previous subsections:
KCE-2: Boolean Flags
As pointed out before, the boolean parameters appear plentifully in the Libbitcoin library APIs. The reason why boolean flags in APIs are problematic has been already discussed.
Table 5 lists the Libbitcoin functions that violate guideline KCE-2-2 and are used in one of the evaluated projects. This subsection describes how the evaluated projects pass boolean arguments to those functions. While using an enumeration, as described before, would often be preferable, using a meaningful named variable is the best case in the given situation. The worse, but tolerable, case is a comment at the call site indicating the meaning of the parameter. The worst cases are situations where the code surrounding the call site needs to be analysed to understand why true or false was passed to a function. The ultimate worst case is where a boolean literal is passed and software developers working on the code must look up the API documentation or an APIs implementation to understand what a boolean flag is used for.
authenticator::apply is used in heartbeat_service::bind, query_service::bind, block_service::bind and transaction_service::bind, both in Libbitcoin server (see Listing 20) and Metaverse. Each of these services is instantiated twice in server_node, once for each value of the boolean flag. While the flag’s name might make the flag’s meaning obvious enough, its apparent security-relevance suggests making it more obvious with the help of an enumeration. However, the examined usages were non-critical.
block_chain::get_top is used in full_node::handle_running in the Libbitcoin node (see Listing 21). In the evaluated code, the flag’s meaning is made obvious through the naming of the additional parameter passed to block_chain::get_top. block_chain::fetch_block is used in blockchain::fetch_block_by_hash and blockchain::fetch_block_by_height in Libbitcoin server, as well as in protocol_block_out::send_next_data of Libbitcoin node. The fetch_block method of the safe_chain class, which block_chain derives from, is used both in the Libbitcoin server and the Libbitcoin node. In the Libbitcoin server, the flag’s meaning is made obvious with the help of an appropriately named local variable whose value is calculated right before block_chain::fetch_block is executed. In the Libbitcoin node, boolean literals are passed whose purpose can merely be derived from the case label related to the blocks the methods are called in.
Listing 20. Usages of authenticator::apply in the Libbitcoin server (truncated). |
> find Libbitcoin-server -name ’*.cpp’ -exec find-api-usage -function-call=’authenticator::apply’ {} + .../src/services/query_service.cpp:88: this->authenticator_.apply(router, domain, this->secure_) .../src/services/block_service.cpp:96: this->authenticator_.apply(xpub, domain, this->secure_) ...
|
Listing 21. Usages of block_chain::get_top in Libbitcoin node. |
> find Libbitcoin-node -name ’*.cpp’ -exec find-api-usage -function-call=’block_chain::get_top’ {} + .../src/full_node.cpp:117: this->chain_.get_top(top_confirmed, false) .../src/full_node.cpp:130: this->chain_.get_top(top_candidate, true)
|
block_chain::fetch_transaction requires two flags to be passed and is used in blockchain::fetch_transaction, blockchain::fetch_transaction1, transaction_pool::fetch_transaction and transaction_pool::fetch_transaction2 in Libbitcoin server. The meaning of the flags is merely described in the examined code through some comments above the method invocation (see Listing 22). Without consulting the method’s declaration, it is not obvious that the comment above the method invocation is related to these two boolean flags. Furthermore, comments always pose the risk of diverging with the code they describe. In the Libbitcoin node, block_chain::fetch_transaction is used in protocol_transaction_out::send_next_data with boolean literals. Again, the purpose of the boolean flags passed can merely be derived from the case label related to the blocks the methods are called in.
Listing 22. Use of boolean flags in block_chain::fetch_transaction. |
/* * The response is restricted to the * confirmed transactions. * * This response excludes witness data * so as not to break old the parsers. */ node.chain().fetch_transaction( hash, true, false, std::bind( &blockchain::transaction_fetched, _1, _2, _3, _4, request, handler));
|
parse_signature is used in input_validate::invoke in Libbitcoin explorer, in sort_multi_sigs and signmultisigtx::invoke in Metaverse and in get_ec_signature in Joinparty. In input_validate::invoke, the flag is passed using a local variable that is initialised with a boolean literal and whose name gives a hint about its use. In both sort_multi_sigs and signmultisigtx::invoke, local variables are used that are initialised right before the function call. In get_ec_signature, a boolean literal is passed without further comment about the argument’s purpose.
In both the Libbitcoin explorer and Metaverse, the boolean flag to decode_base10 is not passed explicitly but its default value is used. In the Libbitcoin explorer, ec_public::ec_public is used in ec_add::invoke, ec_decompress::invoke, ec_multiply::invoke, ec_to_public::invoke and ek_public_to_ec::invoke. In three cases, locally initialised and appropriately named variables are used to pass the flag. In ec_multiply::invoke, the flag is passed by calling a function that indicates the flag’s purpose. In ec_decompress::invoke, a boolean literal is passed and the class name ec_decompress is the only indication of what the flag’s purpose might be. In Metaverse, ec_public::ec_public is used in getnewaddress::invoke, which just passes a boolean literal. While there is a code comment right before the constructor call, it is not obvious that it relates to the boolean flag.
wallet::sign_message is used in get_encoded_signed_message in Joinparty. A boolean literal is passed to wallet::sign_message without any indication about the argument’s purpose. create_key_pair is used in ek_address::invoke, ek_new::invoke and commands::ek_public::invoke in Libbitcoin explorer (see Listing 23). In all three cases, a locally initialised and appropriately named variable is used to pass the flag.
Listing 23. Usages of create_key_pair in Libbitcoin explorer (truncated). |
> find Libbitcoin-explorer -name ’*.cpp’ -exec find-api-usage -function-call=’create_key_pair’ {} + .../src/commands/ek-new.cpp:49: create_key_pair(secret, unused, ...) .../src/commands/ek-public.cpp:52: create_key_pair(unused1, key, ...) .../src/commands/ek-address.cpp:50: create_key_pair(unused, point, ...)
|
ec_private::ec_private is used in ec_to_wif::invoke in Libbitcoin explorer. A locally initialised and appropriately named variable is used to pass the flag.
Both the Libbitcoin explorer and Metaverse use deserialize in operator>> related to the byte class. In both cases, the flag is passed as a boolean literal without any further hint of what the flag’s meaning might be. In the Libbitcoin explorer, split is only used without its flag parameter, which is initialised from its default value. Metaverse, as well, uses split with the flag’s default value into many places. However, in Metaverse, there are also multiple cases where boolean literals are explicitly passed to split. While the flag’s purpose is further commented in one case, in all other cases its meaning is not apparent.
property_tree is used in fetch_block::invoke, fetch_tx::invoke, fetch_utxo::invoke and tx_decode::invoke in Libbitcoin explorer. In all cases, a local variable is used to pass the flag, which is initialised close to the function invocation. In each case, an enumeration is used to initialise this local variable, albeit one with three possible values.
transaction::serialized_size is called in Wallet::create_and_broadcast_transaction and Wallet::create_coin_join_transaction in Joinparty. In both cases, no arguments are explicitly passed to transaction::serialized_size and the method’s default arguments are used instead. In all places where script::from_data is used in Metaverse and Joinparty, its flag option is passed as a boolean literal without any further hints about its purpose.
payment_record::to_data is used in blockchain::history_fetched and blockchain::stealth_fetched in Libbitcoin server. In both cases, the flag is passed as a boolean literal without any further hints about its purpose.
initialize, a free function related to Libbitcoin’s logging functionality. While its flag option is passed as a named local variable in both Libbitcoin node and Libbitcoin server, it is passed as a boolean literal without any further comment about its function in Libbitcoin explorer.
KC-1: Unhandled Error Results
In contrast to exceptions, boolean error results and error result codes are easier to ignore, be it on purpose or by accident. This study, therefore, examined the usage of functions and methods flagging success or failure by returning a boolean value or error code of type
std::error_code (used as code in Libbitcoin with the help of a typedef).
Table 6 lists some of those functions (The full set of functions is available upon request should the corresponding author is contacted) and methods of Libbitcoin’s public API that fall into this category and are used in one of the evaluated projects.
In multiple cases in the Libbitcoin explorer and Metaverse, the return value for secret_to_public is not checked. In Joinparty, the results of secret_to_public calls are always ignored. secret_to_public internally calls secp256k1_ec_pubkey_create and serialize, both of which can fail. Of the three overloads of Bitcoin_uri::set_address, only one can fail, namely, the one which is called with a std::string argument. This overload is used in the Libbitcoin explorer without checking its return value (see Listing 24).
Listing 24. Usage of Bitcoin_uri::set_address in Libbitcoin explorer. |
> find Libbitcoin-explorer -name ’*.cpp’ -exec find-api-usage -function-call=’Bitcoin_uri::set_address’ {} + .../src/commands/uri-encode.cpp:43: uri.set_address(address)
|
create_key_pair, create_token, encrypt and sign_message are used in the lib-Bitcoin explorer without their return values being checked in any of the calls. sign_message is also called in Joinparty, where its return value is properly evaluated.
point::from_data is called in one instance as a call to output_point::from_data (output_point derives from point) in the Libbitcoin server and Metaverse without its return value is checked. While the result of calling decode_base160 is checked in one case in Joinparty, it is ignored in three other cases. Furthermore, none of the calls to decode_base64 in Joinparty is checked for a failed result.
parse_signature, encode_signature, ec_add, and ec_mulitply are all used in Joinparty without their return values ever being evaluated.
In the examined client applications, the return values are checked for invocations of all other functions listed in
Table 6. The invocation of any API functions and methods within Libbitcoin’s libraries themselves have not been further investigated. However, doing so would be advisable, as ignored error result flags are a potential source of security-critical bugs.
KC-2/RU-3: BC_PROPERTY_GET_REF
As demonstrated before, the use of the BC_PROPERTY_GET_REF macro results in property accessors that provide write access to private member variables. However, none of the evaluated applications makes use of the property accessors defined through the BC_PROPERTY_GET_REF macro. Instead, these property accessors seem to be only used by other public API methods of Libbitcoin’s printer class. Consequently, the BC_PROPERTY_GET_REF macro should not only be changed to return a constant reference but the property accessors could even be implemented with private accessibility.
KHS-3: Deprecated Functions
Table 7 lists all of Libbitcoin’s functions and methods that are labelled deprecated using a comment, and which are used by some of the examined client applications. The
BC_DEPRECATED macro defined in
, which should be preferred for tagging deprecated functions, is not used anywhere in Libbitcoin. Furthermore, Mosqueira-Rey et al. (2018) suggest that deprecated API elements should have accompanying documentation explaining the reasons for the deprecation and proposing viable alternatives [
29]. None of the functions listed in
Table 7 has such accompanying documentation.