पीवीएस-स्टूडियो विश्लेषक द्वारा टेलीग्राम ओपन नेटवर्क कोड का सत्यापन

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

परिचय

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

परियोजना CMake विधानसभा प्रणाली का उपयोग करती है, इसलिए विधानसभा और सत्यापन के साथ कोई विशेष समस्याएं नहीं थीं। कोड C ++ 14 में लिखा गया है और इसमें 210 हजार लाइनें हैं:

यह परियोजना छोटी और उच्च गुणवत्ता की है, इसलिए इसमें बहुत अधिक त्रुटियां नहीं थीं, लेकिन वे ध्यान और सुधार के योग्य हैं।

रिटर्न कोड

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; .... } 

PVS-Studio चेतावनी: 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) << ....; } 

PVS-Studio चेतावनी: 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-Studio चेतावनी: 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; .... } 

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

एक नकारात्मक संख्या को दाईं ओर स्थानांतरित करने वाली बिटवाइज़ का संचालन एक अनिर्दिष्ट व्यवहार है: यह ज्ञात नहीं है कि इस मामले में संकेत कैसे व्यवहार करेगा - क्या यह शून्य का विस्तार या भर जाएगा?

नए के बाद शून्य की जाँच करें

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

PVS-Studio चेतावनी: V668 'c' पॉइंटर को null के परीक्षण में कोई समझदारी नहीं है, क्योंकि 'new' ऑपरेटर का उपयोग करके मेमोरी आवंटित की गई थी। मेमोरी आवंटन त्रुटि के मामले में अपवाद उत्पन्न होगा। 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); } } .... } 

PVS-Studio चेतावनी: 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-Studio चेतावनी: 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; } } 

PVS-Studio चेतावनी: 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)); .... } 

डिफाइन_बुलेटिन फ़ंक्शन में, आप 6 = के बराबर मोड पैरामीटर के साथ compile_cmp_int कॉल कर सकते हैं।

खैर, दूसरा वही संकलन_ 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, "") // <= 

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

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

निष्कर्ष में

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

यदि आप इस लेख को अंग्रेजी बोलने वाले दर्शकों के साथ साझा करना चाहते हैं, तो कृपया अनुवाद के लिंक का उपयोग करें: सर्गेई लारिन। पीवीएस-स्टूडियो के साथ टेलीग्राम ओपन नेटवर्क की जाँच ।