पीवीएस-स्टूडियो के साथ टेलीग्राम ओपन नेटवर्क की जाँच

टेलीग्राम ओपन नेटवर्क (TON) उसी टीम द्वारा एक प्लेटफ़ॉर्म है जिसने टेलीग्राम मैसेंजर विकसित किया। ब्लॉकचेन के अलावा, TON सेवाओं का एक बड़ा सेट प्रदान करता है। डेवलपर्स ने हाल ही में प्लेटफ़ॉर्म का कोड बनाया, जो C ++ में लिखा है, सार्वजनिक रूप से उपलब्ध है और इसे GitHub पर अपलोड किया गया है। हमने आधिकारिक रिलीज से पहले परियोजना की जांच करने का फैसला किया।

परिचय

टेलीग्राम ओपन नेटवर्क विभिन्न सेवाओं का एक समूह है। अन्य बातों के अलावा, यह ग्राम क्रिप्टोक्यूरेंसी के आधार पर अपनी स्वयं की एक भुगतान प्रणाली प्रदान करता है, और एक आभासी मशीन जिसे टॉन वीएम कहा जाता है, जो स्मार्ट अनुबंधों को निष्पादित करता है। यह एक संदेश सेवा, TON संदेश भी प्रदान करता है। संपूर्ण रूप से परियोजना को इंटरनेट सेंसरशिप के लिए एक प्रतिवाद के रूप में देखा जाता है।

प्रोजेक्ट CMake के साथ बनाया गया है, इसलिए मुझे इसके निर्माण और जाँच में कोई कठिनाई नहीं हुई। स्रोत कोड C ++ 14 में लिखा गया है और 210 हजार LOC तक चलता है:

चूंकि यह परियोजना एक छोटी और उच्च गुणवत्ता वाली है, इसलिए इसमें कई कीड़े नहीं हैं, लेकिन फिर भी इनसे निपटा जाना चाहिए।

रिटर्न कोड

static int process_workchain_shard_hashes(....) { .... if (f == 1) { if ((shard.shard & 1) || cs.size_ext() != 0x20000) { return false; // <= } .... int r = process_workchain_shard_hashes(....); if (r < 0) { return r; } .... return cb.store_bool_bool(true) && cb.store_ref_bool(std::move(left)) && cb.store_ref_bool(std::move(right)) && cb.finalize_to(branch) ? r : -1; .... } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V601 'गलत' मान को पूर्णांक प्रकार में डाला जाता है। mc-config.cpp 884

ऐसा लगता है कि फ़ंक्शन गलत प्रकार की त्रुटि स्थिति यहां लौटाता है। फ़ंक्शन को स्पष्ट रूप से सही / गलत के बजाय विफलता के लिए नकारात्मक मान वापस करना चाहिए। यह कम से कम कोड में आगे क्या करता है, जहां यह -1 देता है।

एक चर की तुलना अपने आप से करना

 class LastBlock : public td::actor::Actor { .... ton::ZeroStateIdExt zero_state_id_; .... }; void LastBlock::update_zero_state(ton::ZeroStateIdExt zero_state_id) { .... if (zero_state_id_ == zero_state_id_) { return; } LOG(FATAL) << ....; } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V501 बाईं और '==' ऑपरेटर के दाईं ओर समान उप-अभिव्यक्तियाँ हैं: zero_state_id_ == zero_state_id_ LastBlock.cpp 66

TON एक कोडिंग मानक का अनुसरण करता है जो यह बताता है कि कक्षा सदस्यों के नाम एक अंडरस्कोर में समाप्त होने चाहिए। हालाँकि, इस तरह के मामलों में, यह अंकन बग को जन्म दे सकता है क्योंकि आप अंडरस्कोर को अनदेखा करने का जोखिम उठाते हैं। इस फ़ंक्शन को दिए गए तर्क का नाम कक्षा के सदस्य के समान है, जो उन्हें मिश्रण करना आसान बनाता है। यह तर्क है कि तुलना में भाग लेने के लिए सबसे अधिक संभावना थी।

असुरक्षित मैक्रो

 namespace td { namespace detail { [[noreturn]] void process_check_error(const char *message, const char *file, int line); } // namespace detail } #define CHECK(condition) \ if (!(condition)) { \ ::td::detail::process_check_error(#condition, __FILE__, __LINE__); \ } void BlockDb::get_block_handle(BlockIdExt id, ....) { if (!id.is_valid()) { promise.set_error(....); return; } CHECK(id.is_valid()); // <= .... } 

PVS- स्टूडियो डायग्नोस्टिक संदेश: V581 एक-दूसरे के साथ स्थित 'if' स्टेटमेंट्स की सशर्त अभिव्यक्तियाँ समान हैं। चेक लाइनें: 80, 84. blockdb.cpp 84

CHECK मैक्रो के अंदर की स्थिति कभी भी निष्पादित नहीं होगी क्योंकि यह पहले से ही पूर्व में बयान द्वारा जाँच की जा चुकी है।

यहां एक और त्रुटि भी मौजूद है: CHECK मैक्रो असुरक्षित है क्योंकि इसके अंदर की स्थिति एक {{} निर्माण में लिपटी नहीं है (0) । अन्य शाखा में अन्य स्थितियों के साथ टकराव से बचने के लिए इस तरह के रैपिंग की आवश्यकता होती है। दूसरे शब्दों में, निम्नलिखित कोड उम्मीद के मुताबिक काम नहीं करेगा:

 if (X) CHECK(condition) else foo(); 

एक हस्ताक्षरित चर की जाँच करना

 class Slice { .... char operator[](size_t i) const; .... }; td::Result<int> CellSerializationInfo::get_bits(td::Slice cell) const { .... int last = cell[data_offset + data_len - 1]; if (!last || last == 0x80) { // <= return td::Status::Error("overlong encoding"); } .... } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V560 सशर्त अभिव्यक्ति का एक हिस्सा हमेशा गलत होता है: अंतिम == 0x80। boc.cpp 78

स्थिति का दूसरा भाग कभी भी निष्पादित नहीं करेगा क्योंकि इस मामले में टाइप चार पर हस्ताक्षर किए गए हैं। वैल्यू ऑफ टाइप इंट के एक वैरिएबल को असाइन करते समय, साइन एक्सटेंशन हो जाएगा, इसलिए इसके मान अभी भी सीमा [-128, 127] के भीतर होंगे, न कि [0, 256]।

यह ध्यान दिया जाना चाहिए कि चार हमेशा हस्ताक्षरित नहीं होते हैं: इसका व्यवहार प्लेटफॉर्म है- और संकलक-निर्भर। इसलिए सिद्धांत रूप में, एक अलग मंच पर निर्माण के दौरान प्रश्न की स्थिति अभी भी निष्पादित हो सकती है।

एक नकारात्मक संख्या को शिफ्ट करना

 template <class Tr> bool AnyIntView<Tr>::export_bits_any(....) const { .... int mask = (-0x100 >> offs) & 0xff; .... } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: वी 610 अनिर्दिष्ट व्यवहार। शिफ्ट ऑपरेटर '>>' की जाँच करें। बाएं ऑपरेंड '-0x100' नकारात्मक है। bigint.hpp 1925

एक ऋणात्मक संख्या पर एक बिटवाइज़ राइट शिफ्ट ऑपरेशन को निष्पादित करना अनिर्दिष्ट व्यवहार है: यदि अग्रिम में संकेत को बढ़ाया जाएगा या शून्य के साथ गद्देदार किया जाएगा, तो यह जानना असंभव है।

नए के बाद नल की जांच

 CellBuilder* CellBuilder::make_copy() const { CellBuilder* c = new CellBuilder(); if (!c) { // <= throw CellWriteError(); } .... } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V668 'सी' पॉइंटर को अशक्त करने के परीक्षण में कोई मतलब नहीं है, क्योंकि स्मृति को 'नए' ऑपरेटर का उपयोग करके आवंटित किया गया था। मेमोरी आवंटन त्रुटि के मामले में अपवाद उत्पन्न होगा। CellBuilder.cpp 531

संदेश यह सब कहता है: यदि स्मृति आवंटन विफल रहता है, तो प्रोग्राम एक अशक्त सूचक को वापस करने के बजाय एक अपवाद फेंक देगा। इसका मतलब है कि जाँच व्यर्थ है।

निरर्थक जाँच

 int main(int argc, char* const argv[]) { .... if (!no_env) { const char* path = std::getenv("FIFTPATH"); if (path) { parse_include_path_set(path ? path : "/usr/lib/fift", source_include_path); } } .... } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V547 एक्सप्रेशन 'पाथ' हमेशा सही होता है। fift-main.cpp 136

यह स्निपेट परियोजना की आंतरिक उपयोगिताओं में से एक से लिया गया है। टर्नरी ऑपरेटर इस मामले में निरर्थक है: जिस स्थिति की वह जाँच करता है वह पहले से ही पूर्व में दिए गए विवरण द्वारा जाँच की जाती है। ऐसा लगता है कि डेवलपर्स इस टर्नरी ऑपरेटर को हटाने के लिए भूल गए जब उन्होंने मानक पथों के उपयोग को छोड़ने का फैसला किया (सहायता संदेश में उन लोगों का कम से कम कोई उल्लेख नहीं है)।

अप्रयुक्त चर

 bool Op::set_var_info_except(const VarDescrList& new_var_info, const std::vector<var_idx_t>& var_list) { if (!var_list.size()) { return set_var_info(new_var_info); } VarDescrList tmp_info{new_var_info}; tmp_info -= var_list; return set_var_info(new_var_info); // <= } 

PVS- स्टूडियो डायग्नोस्टिक संदेश: V1001 'tmp_info' वेरिएबल असाइन किया गया है, लेकिन फ़ंक्शन के अंत तक उपयोग नहीं किया गया है। विश्लेषक

डेवलपर्स स्पष्ट रूप से इस फ़ंक्शन की अंतिम पंक्ति में tmp_info नामक एक चर का उपयोग करने जा रहे थे। यहाँ उसी फ़ंक्शन का कोड है, लेकिन अन्य पैरामीटर विनिर्देशक के साथ:

 bool Op::set_var_info_except(VarDescrList&& new_var_info, const std::vector<var_idx_t>& var_list) { if (var_list.size()) { new_var_info -= var_list; // <= } return set_var_info(std::move(new_var_info)); } 

ग्रेटर या इससे कम?

 int compute_compare(const VarDescr& x, const VarDescr& y, int mode) { switch (mode) { case 1: // > return x.always_greater(y) ? 1 : (x.always_leq(y) ? 2 : 3); case 2: // = return x.always_equal(y) ? 1 : (x.always_neq(y) ? 2 : 3); case 3: // >= return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3); case 4: // < return x.always_less(y) ? 1 : (x.always_geq(y) ? 2 : 3); case 5: // <> return x.always_neq(y) ? 1 : (x.always_equal(y) ? 2 : 3); case 6: // >= return x.always_geq(y) ? 1 : (x.always_less(y) ? 2 : 3); case 7: // <=> return x.always_less(y) ? 1 : (x.always_equal(y) ? 2 : (x.always_greater(y) ? 4 : (x.always_leq(y) ? 3 : (x.always_geq(y) ? 6 : (x.always_neq(y) ? 5 : 7))))); default: return 7; } } 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V1037 दो या दो से अधिक केस-शाखाएँ एक ही क्रिया करते हैं। लाइनों की जाँच करें: 639, 645 बिल्ट। 639

यदि आप ध्यान से पढ़ते हैं, तो आपने देखा कि इस कोड में <= ऑपरेशन का अभाव है। वास्तव में, यह इस ऑपरेशन है कि 6 के मामले से निपटना चाहिए। हम दो धब्बों को देखकर इसे घटा सकते हैं। पहला इनिशियलाइज़ेशन कोड है:

 AsmOp compile_cmp_int(std::vector<VarDescr>& res, std::vector<VarDescr>& args, int mode) { .... if (x.is_int_const() && y.is_int_const()) { r.set_const(compute_compare(x.int_const, y.int_const, mode)); x.unused(); y.unused(); return push_const(r.int_const); } int v = compute_compare(x, y, mode); .... } void define_builtins() { .... define_builtin_func("_==_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 2)); define_builtin_func("_!=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 5)); define_builtin_func("_<_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 4)); define_builtin_func("_>_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 1)); define_builtin_func("_<=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 6)); define_builtin_func("_>=_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 3)); define_builtin_func("_<=>_", arith_bin_op, std::bind(compile_cmp_int, _1, _2, 7)); .... } 

जैसा कि आप देख सकते हैं, define_builtins फ़ंक्शन, में 6 = के लिए सेट मोड पैरामीटर के साथ <= ऑपरेटर के लिए एक कॉल compile_cmp_int शामिल है।

दूसरा स्थान कंपाइल_कम्प_इंट फंक्शन ही है, जो संचालन के नामों को सूचीबद्ध करता है:

 AsmOp compile_cmp_int(std::vector<VarDescr>& res, std::vector<VarDescr>& args, int mode) { .... static const char* cmp_names[] = {"", "GREATER", "EQUAL", "GEQ", "LESS", "NEQ", "LEQ", "CMP"}; .... return exec_op(cmp_names[mode], 2); } 

इंडेक्स 6 LEQ शब्द से मेल खाता है, जिसका अर्थ है "कम या समान।"

यह तुलनात्मक कार्यों में पाए जाने वाले बगों के वर्ग का एक और अच्छा बग है।

विविध

 #define VM_LOG_IMPL(st, mask) \ LOG_IMPL_FULL(get_log_interface(st), ...., VERBOSITY_NAME(DEBUG), \ (get_log_mask(st) & mask) != 0, "") // <= 

पीवीएस-स्टूडियो डायग्नोस्टिक संदेश: V1003 मैक्रो 'VM_LOG_IMPL' एक खतरनाक अभिव्यक्ति है। पैरामीटर 'मास्क' को कोष्ठकों से घिरा होना चाहिए। log.h 23

VM_LOG_IMPL मैक्रो असुरक्षित है। इसका दूसरा पैरामीटर कोष्ठकों में संलग्न नहीं है, जो संभावित रूप से अवांछनीय दुष्प्रभाव पैदा कर सकता है यदि एक जटिल अभिव्यक्ति को स्थिति में पारित किया जाता है। लेकिन अगर मुखौटा सिर्फ एक स्थिर है, तो यह कोड बिना किसी समस्या के चलेगा। उस ने कहा, कुछ भी नहीं आप और कुछ भी मैक्रो को पारित करने से रोकता है।

निष्कर्ष

TON बहुत छोटा निकला, इसलिए वहाँ खोजने के लिए कुछ कीड़े हैं, जिन्हें टेलीग्राम डेवलपर टीम को निश्चित रूप से श्रेय दिया जाना चाहिए। लेकिन हर कोई हर बार गलती करता है, यहां तक ​​कि ये लोग भी। कोड एनालाइजर सबसे शक्तिशाली कोड बेस में भी प्रारंभिक विकास चरणों में स्रोत कोड में खतरनाक स्पॉट का पता लगाने में सक्षम शक्तिशाली उपकरण हैं, इसलिए उन्हें उपेक्षित न करें। स्थैतिक विश्लेषण का मतलब समय-समय पर चलना नहीं है, बल्कि विकास प्रक्रिया का हिस्सा होना चाहिए: "प्रक्रिया में स्थैतिक विश्लेषण का परिचय दें, इसके साथ कीड़े की खोज न करें "।