التحقق من كود Telegram Open Network بواسطة محلل PVS-Studio

Telegram Open Network (TON) هي عبارة عن منصة من المبدعين من Telegram messenger ، والتي ، بالإضافة إلى blockchain ، تحتوي على مجموعة كبيرة من الخدمات. قام المطورون مؤخرًا بنشر رمز النظام الأساسي المكتوب بلغة C ++ ونشروه على GitHub. أردنا التحقق من المشروع قبل الإطلاق الرسمي.

مقدمة

Telegram Open Network هي مجموعة من الخدمات المختلفة. على سبيل المثال ، يحتوي المشروع على نظام الدفع الخاص به استنادًا إلى العملة المشفرة Gram ، وهناك أيضًا جهاز افتراضي TON VM - تعمل العقود الذكية عليه. هناك خدمة الرسائل - رسائل تون. يهدف المشروع نفسه إلى مكافحة الرقابة على الإنترنت.

يستخدم المشروع نظام التجميع 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 يتم رسم القيمة "false" ضمنيًا على نوع العدد الصحيح. 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 غير آمن ، لأن الشرط بداخلها غير ملفوف في do {...} بينما (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"); } .... } 

تحذير PVS-Studio: V560 جزء من التعبير الشرطي خطأ دائمًا: last == 0x80. boc.cpp 78

الجزء الثاني من الحالة لن يتحقق أبداً ، لأن اكتب شار في هذه الحالة لديه علامة. عند تعيين قيمة لمتغير type int ، سيتم توسيع العلامة ، وبالتالي سيظل نطاق قيم المتغير في الحد [-128 ، 127] وليس في [0 ، 256].

تجدر الإشارة إلى أن نوع char لن يتم توقيعه دائمًا - يعتمد هذا السلوك على النظام الأساسي والمترجم. لذلك لا يزال من الممكن تحقيق هذا الشرط نظريًا عند التجميع على منصة أخرى.

بت قليلا بت التحول

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

تحذير PVS-Studio: V610 سلوك غير محدد. تحقق من مشغل التحول '>>'. المعامل الأيسر '-0x100' سلبي. bigint.hpp 1925

إن عملية نقل الرقم السلبي إلى اليمين بطريقة غير محددة هي سلوك غير محدد: من غير المعروف كيف ستتصرف الإشارة في هذه الحالة - هل ستتوسع أو تملأ الأصفار؟

تحقق من عدم وجود بعد جديد

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

تحذير PVS-Studio: V668 لا يوجد أي معنى في اختبار المؤشر "c" ضد قيمة خالية ، حيث تم تخصيص الذاكرة باستخدام عامل التشغيل "الجديد". سيتم إنشاء الاستثناء في حالة خطأ تخصيص الذاكرة. 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 Expression 'path' صحيح دائمًا. 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' ولكن لا يتم استخدامه بحلول نهاية الوظيفة. analyser.cpp 140

من الواضح أن السطر الأخير من هذه الوظيفة خطط لاستخدام المتغير 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 buildins.cpp 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 ، يمكنك رؤية استدعاء compile_cmp_int لـ <= مع معلمة الوضع تساوي 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, "") // <= 

تحذير PVS-Studio: V1003 الماكرو 'VM_LOG_IMPL' تعبير خطير. يجب أن تكون المعلمة 'قناع' محاطة بأقواس. log.h 23

الماكرو VM_LOG_IMPL غير آمن. المعلمة الثانية ليست محاطة بأقواس - وهذا يمكن أن يؤدي إلى آثار جانبية إذا تم تمرير تعبير معقد إلى الحالة. إذا كان القناع ثابتًا ، فلن تكون هناك مشكلات في هذا الرمز ، لكن لا شيء يمنعنا من نقل شيء آخر إلى الماكرو.

في الختام

تبين أن رمز TON ليس كبيرًا جدًا ، ولا توجد الكثير من الأخطاء. وهو أمر يستحق بالطبع الإشادة بالمبرمجين من فريق Telegram. ومع ذلك ، حتى أنهم ليسوا في مأمن من الأخطاء. أداة تحليل الشفرات هي أداة قوية قادرة على إيجاد أماكن خطيرة في المراحل المبكرة حتى في قواعد الشفرة عالية الجودة ، لذلك يجب عدم إهمال استخدامها. لا يعد التحليل الثابت أداة للفحوصات لمرة واحدة ، ولكنه جزء من عملية التطوير: " تضمين التحليل الثابت في العملية ، ولا تبحث عن أخطاء بها ".

إذا كنت ترغب في مشاركة هذه المقالة مع جمهور يتحدث الإنجليزية ، فالرجاء استخدام الرابط الخاص بترجمة: Sergey Larin. التحقق من شبكة Telegram Open باستخدام PVS-Studio .