11 KiB
21 — تدقيق سيرو: العيوب الأمنية والمشاكل المرصودة
سجلّ قابل للمعالجة لكل ما رُصد في كود سيرو الحيّ (
~/development/App/Siro) أثناء بنائنا لـTripz. الغرض مزدوج: (1) ألّا نكرّرها في Tripz، (2) قائمة إصلاحات جاهزة لسيرو نفسه حين نعالجه.كل ما يلي في سيرو (PHP/MySQL) لا في Tripz. حيث أصلحنا المكافئ في Tripz، أُشير إليه. الخطورة: 🔴 حرجة · 🟠 متوسطة · 🟡 منخفضة/جودة.
آخر تحديث: 2026-07-24 (S18 تُصحّح: ليست عيباً، هي سلوك صحيح).
المدفوعات والسحب (payment_server/ · request_payout.php · finalizePayout)
| # | الخطورة | العيب | الإصلاح المقترح لسيرو | حالنا في Tripz |
|---|---|---|---|---|
| S1 | 🔴 | IDOR في السحب: request_payout.php يأخذ driverId/phone من جسم الطلب لا من التوكن — سائق يسحب رصيد غيره إلى هاتفه. |
اشتقاق هوية السائق من JWT الموقَّع حصراً. | ✅ نظيف — payouts.request يستعمل user.userId من التوكن. |
| S2 | 🔴 | لا حجز رصيد عند طلب السحب → طلبات متزامنة تمرّ كلها = double-spend. | حجز ذرّي عند التأكيد قبل أي تحويل. | ✅ خصم ذرّي عند confirm (I1/I4). |
| S3 | 🔴 | رسم 3500 يناقض نفسه: الفحص يتطلب amount + 3500 والإنهاء يخصم amount - 3500 = تسرّب مال. الرقم مكتوب يدوياً مرّتين. |
ثابت واحد + اختبار يطابق الفحص بالخصم. | ✅ لا رسم مكرّر؛ العمولة من الرصيد التشغيلي (docs/18). |
| S4 | 🔴 | finalizePayout يعمل 5 كتابات بلا معاملة/تراجع → فشل في المنتصف يترك سحباً نصف مُسوّى. |
لفّ الكتابات في transaction واحدة. | ✅ الخصم والقيد في معاملة واحدة (I1). |
| S5 | 🟠 | driverWallet.amount نوعه varchar(10) — المال مخزَّن نصّاً، وSUM() على نصوص. |
تحويله DECIMAL(12,3). |
✅ numeric(12,3) + CHECK (balance >= 0). |
| S6 | 🟠 | لا OTP على السحب — phone_verification موجود لكنه مربوط بالتسجيل/الدخول فقط. |
OTP + تحقّق قبل التحويل. | ✅ OTP على السحب عبر المُوزِّع (I4). |
| S7 | 🟡 | جدولان متداخلان لنفس المفهوم: payout_requests وdriver_withdrawal_requests. |
توحيدهما في جدول واحد. | ✅ جدول pay_payouts واحد. |
✅ إعادة فحص سيرو — 2026-07-24 (بطلب المالك: «العيوب في المدفوعات عدّلناها»)
فُحص الكود مباشرة. 4 من 7 مُصلَحة فعلاً وبجودة جيدة، و3 ما زالت مفتوحة + عيب جديد.
| # | الحالة اليوم في سيرو | الدليل |
|---|---|---|
| S1 IDOR | ✅ مُصلَح | request_payout.php:9 → $driverId = $decodedToken->sub، والهاتف يُقرأ من driver بالـid لا من الطلب |
| S2 حجز الرصيد | ✅ مُصلَح | beginTransaction + SELECT … FOR UPDATE + حالة payout_reserved + rollBack |
| S3 رسم 3500 | ✅ مُصلَح | الرقم اختفى من مسار السحب كلياً |
| S6 OTP على السحب | ✅ مُصلَح | request_payout.php:28,60–72 — OTP إلزامي ويُتحقق من token_verification_driver |
| S4 معاملة في الإنهاء | ✅ مُصلَح (2026-07-24) | finalize_payout.php — الكتابات 2–6 داخل beginTransaction/commit واحدة، مع rollBack في catch. فُحصت أثناء الإصلاح: $encryptionHelper كان يُستعمل داخل الدالة بلا global (متغيّر غير مُعرَّف في نطاق الدالة) → كل استدعاء كان يفشل عند الخطوة 1 قبل أي كتابة. أُصلح بإضافة global $encryptionHelper; |
S5 varchar(10) |
✅ مُصلَح (2026-07-24) | driverWallet/siroWallet في WalletDB.sql + seferWallet (جدول ثالث لم يكن مذكوراً صراحة، وُجد أثناء الإصلاح) في schema_primary.sql + schema_ride.sql (نسختان) ← DECIMAL(12,3). هجرتان في payment_server/migrations/ وbackend/migrations/. لم يُصلَح: backend/ride/seferWallet/add.php:28 يُدخل $amount بلا is_numeric() — بند منفصل جديد |
| S7 الجدولان | 🟡 ما زال مفتوحاً | driver_withdrawal_requests قائم بجانب payout_requests (WalletDB.sql:132) |
✅ S18 — ليست مشكلة (توضيح تصميمي، 2026-07-24)
finalize_payout.php الخطوة 6:
UPDATE payments SET isGiven = TRUE WHERE driverID = :driverId AND isGiven = FALSE
الملاحظة الأصلية (تُصحّح): بدا أنه يعلّم كل الدفعات بغضّ النظر عن مبلغ السحب.
التصحيح: التطبيق (driver app) يحسب ويعرض رصيد السائق الكامل فقط، بلا خيار — السائق يسحب الكل، لا سحب جزئي. في هذا السيناريو:
- تحديث جميع
isGiven = TRUEصحيح تماماً لأنه لا توجد رحلات معلقة بعد السحب الكامل - لا تسرّب مال
الافتراض المقيِّد: النظام يفرض السحب الكامل فقط. إذا تغيّر هذا لاحقاً (السماح بسحب جزئي)، عندها يجب ربط payments بـ payout_id.
✅ البوابة اجتيزت (2026-07-24): S4 و S5 مُصلَحان. بند جديد قبل أول استنساخ: إضافة
is_numeric($amount)فيbackend/ride/seferWallet/add.php— لم يكن جزءاً من S4/S5 الأصليين، اكتُشف أثناء إصلاحهما.
المصادقة وكلمة المرور (auth/)
| # | الخطورة | العيب | الإصلاح المقترح لسيرو | حالنا في Tripz |
|---|---|---|---|---|
| S8 | 🔴 | مقارنة كلمة مرور نصّية: loginUsingCredentialsWithoutGoogle.php سطر 114 — `password_verify(...) |
$password === $data['password']`. الشقّ الثاني يتجاوز الـhash كلياً، ويطابق كلمة المرور المخزَّنة نصّاً لو كانت كذلك. | |
| S9 | 🟠 | كلمة مرور مشتركة ثابتة 'SiroPassenger2026!' للحسابات التجريبية في الكود. |
نقلها لمتغيّر بيئة على الأقل، والأفضل إزالة المسار. | ✅ لا مسار كلمة مرور. |
| S10 | 🟡 | password = hash($email) وهمي عند التسجيل — حقل بلا معنى يوحي بأمان غير موجود. |
إزالة العمود إن لم يُستعمل. | ✅ لا عمود كلمة مرور. |
| S11 | 🟠 | user_type يُؤخذ من الطلب بلا تحقّق توقيع (تعليقهم صريح: "JWT not trusted without signature verification") — عميل يدّعي admin. يُلطَّف جزئياً بقائمة ADMIN_PHONE_NUMBERS. |
التحقق من توقيع JWT واعتماد الدور منه. | ✅ الدور من التوكن الموقَّع (RolesGuard). |
الـOTP (auth/otp/)
| # | الخطورة | العيب | الإصلاح المقترح لسيرو | حالنا في Tripz |
|---|---|---|---|---|
| S12 | 🟠 | رمز OTP من 3 خانات (1000 احتمال فقط) — قابل للتخمين. | 6 خانات + قفل بعد المحاولات. | ✅ 4 خانات (قابل للرفع) + قفل 5 محاولات (D4). عند النقل نرفعه لـ6. |
| S13 | 🟠 | مسح جدول كامل وفكّ تشفير كل صف للتحقق من الرمز (verify.php: SELECT * ... foreach decrypt) — بطيء ويتسرّب توقيتاً. |
فهرس أعمى (blind index) على الهاتف. | ✅ phone_bidx (HMAC حتمي) للبحث المباشر (docs/16). |
| S14 | 🟡 | مساران متوازيان للـOTP: request.php/verify.php (قاعدة، مشفّر) وOtpService.php (Redis) — تكرار وتضارب. |
توحيد على مسار واحد. | ✅ مسار Redis واحد. |
| S15 | 🟡 | echo لأخطاء المزوّد الخام في sendIntaleqOtp ("Temporarily echo the raw error") — تسريب تشخيصي في الإنتاج. |
إزالة الـecho، لوغ فقط. | ✅ لا echo؛ لوغ خادمي. |
| S16 | 🟡 | تخزين OTP مشفَّراً بـGCM في القاعدة ثم مسح الجدول لفكّه — تكلفة بلا فائدة (الرمز قصير العمر، الأنسب Redis + hash). | Redis + password_hash كما في OtpService. |
✅ Redis مباشرةً. |
التشفير (core/Security/EncryptionHelper.php)
| # | الخطورة | العيب | الإصلاح المقترح لسيرو | حالنا في Tripz |
|---|---|---|---|---|
| S17 | 🔴 | AES-CBC بـIV ثابت (موثَّق سابقاً في docs/16) — نفس النص ينتج نفس الشيفرة، ويسرّب الأنماط. | AES-256-GCM + IV عشوائي لكل قيمة. | ✅ منفَّذ (docs/16). |
حدّ الطلبات (core/Auth/RateLimiter.php) — جيد، للنسخ لا للإصلاح
تصميمه سليم: حدود مسمّاة لكل نوع + fallback بملف عند تعطّل Redis (fail-closed). النقطة الوحيدة: يتتبّع بـIP:userId — جيد، لكن نقاط ما قبل الدخول بالـIP وحده (لا مفرّ). ليس عيباً — مرجع نتعلّم منه.
نقاط تحتاج تأكيد المالك (لم أجزم بها)
- هل
raw_sms_log+ Gemini (تسوية الدفع من رسائل المزوّد) يُدقَّق ضد إعادة الاستعمال (نفس الرسالة تُسوّى مرتين)؟ — يحتاج فحصwebhook_sms/webhook.phpعند معالجة سيرو. - هل توكنات Nabeh/مفاتيح Kazumi في متغيّرات بيئة على كل الخوادم أم مكتوبة في مكان ما؟ — يُتحقَّق عند النقل.
← ذو صلة: 16-encryption · 17-backend-backlog · 18-driver-credit-commission