Skip to content

Latest commit

 

History

History
532 lines (411 loc) · 16.1 KB

File metadata and controls

532 lines (411 loc) · 16.1 KB

Реальные примеры из проектов

Пример 1: Telegram Authentication - Полный анализ

Исходная проблема

В проекте была обнаружена критическая уязвимость в валидации Telegram initData.

Код ДО исправления

// src/utils/telegram.rs (упрощенно)
pub fn validate_telegram_data(
    raw: &str,
    hash: &str,
    bot_token: &str,
    skip_validation: bool,
) -> bool {
    if skip_validation {
        return true;
    }

    let parsed = parse_query_string_raw(raw);

    // ❌ ПРОБЛЕМА 1: НЕТ ПРОВЕРКИ auth_date!
    // Старые данные принимаются как валидные = Replay Attack

    // ❌ ПРОБЛЕМА 2: Двойной парсинг
    let computed_hash = compute_telegram_hash(raw, bot_token);
    //                                        ^^^
    // Внутри compute_telegram_hash() парсится еще раз!

    computed_hash == hash
}

fn compute_telegram_hash(raw: &str, bot_token: &str) -> String {
    let parsed = parse_query_string_raw(raw);  // Парсинг #2 ❌

    let mut kv_pairs: Vec<(String, String)> = parsed.into_iter()...  // ❌ Копирует String
    kv_pairs.sort_by(...);

    let data_check_string = kv_pairs.iter()...

    let secret = Sha256::digest(bot_token.as_bytes());
    let mut mac = HmacSha256::new_from_slice(&secret)
        .expect("HMAC initialization failed");  // ❌ ПРОБЛЕМА 3: expect()!

    // ...
    hex::encode(mac.finalize().into_bytes())
}

Найденные проблемы

1. ❌ КРИТИЧЕСКАЯ УЯЗВИМОСТЬ: Replay Attack

Что не так:

// НЕТ проверки auth_date
// Означает: если злоумышленник перехватит initData, он может
// использовать его ВЕЧНО для аутентификации!

Как эксплуатируется:

  1. Жертва заходит в Telegram Mini App
  2. Злоумышленник перехватывает запрос (MITM, скомпрометированный Wi-Fi)
  3. Сохраняет initData с валидной подписью
  4. Месяц спустя использует эти же данные для входа от имени жертвы

Последствия:

  • Полная компрометация аккаунта
  • CVSS Score: 9.1 (Critical)
  • Нарушение официальной документации Telegram

Доказательство уязвимости:

#[test]
fn test_replay_attack_vulnerability() {
    let bot_token = "real_token";

    // Создаем initData со старой датой (2 года назад)
    let old_timestamp = 1640995200; // 2022-01-01
    let raw = format!("auth_date={}&user=...", old_timestamp);

    // Вычисляем правильный hash
    let hash = compute_telegram_hash(&raw, bot_token);

    // ❌ УЯЗВИМОСТЬ: Валидация проходит для старых данных!
    assert!(validate_telegram_data(&raw, &hash, bot_token, false));

    // Ожидаемое поведение: должна была вернуть false!
}

2. ❌ ПРОИЗВОДИТЕЛЬНОСТЬ: Двойной парсинг

Метрики:

Benchmark: validate_telegram_data (1000 вызовов)
ДО оптимизации:  847.3 µs
ПОСЛЕ:           412.1 µs
Улучшение:       51.4% быстрее ✅

Пояснение:

  • parse_query_string_raw() вызывается 2 раза
  • Для строки в 200 символов это ~100 операций парсинга вместо 50

3. ❌ НАРУШЕНИЕ ПРОТОКОЛА: expect()

AI Development Protocol v2.1 §4:

"Никаких паник, кроме тестов и unreachable!()"

Код:

.expect("HMAC initialization failed")  // ❌ ЗАПРЕЩЕНО!

Почему проблема:

  • В production panic = DOS атака
  • Если злоумышленник подберет данные, вызывающие panic → сервер падает
  • Нарушает принцип graceful degradation

4. ⚠️ ОПТИМИЗАЦИЯ: Избыточное копирование

Было:

let mut kv_pairs: Vec<(String, String)> = parsed.into_iter()
    .filter(|(k, _)| k != "hash" && k != "signature")
    .collect();
// Для 10 пар по 20 символов = ~400 байт копирования

Стало:

let mut kv_pairs: Vec<(&String, &String)> = parsed.iter()
    .filter(|(k, _)| k.as_str() != FIELD_HASH && k.as_str() != FIELD_SIGNATURE)
    .collect();
// Zero-copy! 0 байт копирования

Эффект: 3-5x быстрее создание вектора


Исправленный код

// Константы вместо magic strings
pub const DEFAULT_MAX_AUTH_AGE_SECONDS: u64 = 86400;
const FIELD_AUTH_DATE: &str = "auth_date";
const FIELD_HASH: &str = "hash";
const FIELD_SIGNATURE: &str = "signature";

/// Validates auth_date freshness to prevent replay attacks
fn validate_auth_date_freshness(
    parsed: &BTreeMap<String, String>,
    max_age_seconds: u64,
) -> bool {
    if let Some(auth_date_str) = parsed.get(FIELD_AUTH_DATE) {
        if let Ok(auth_date) = auth_date_str.parse::<u64>() {
            let current_time = get_current_timestamp();
            let age = current_time.saturating_sub(auth_date);

            if age > max_age_seconds {
                tracing::warn!(
                    "auth_date too old: {} seconds (max: {})",
                    age,
                    max_age_seconds
                );
                return false;  // ✅ Защита от replay attack
            }
            return true;
        } else {
            tracing::warn!("Invalid auth_date format: {}", auth_date_str);
            return false;
        }
    }

    tracing::warn!("Missing auth_date in initData");
    false
}

/// Computes Telegram hash - теперь без expect()
fn compute_telegram_hash(
    parsed: &BTreeMap<String, String>,  // ✅ Принимаем ссылку
    bot_token: &str,
) -> Option<String> {  // ✅ Возвращаем Option вместо panic
    let mut kv_pairs: Vec<(&String, &String)> = parsed.iter()  // ✅ Zero-copy
        .filter(|(k, _)| k.as_str() != FIELD_HASH && k.as_str() != FIELD_SIGNATURE)
        .collect();

    kv_pairs.sort_by(|a, b| a.0.cmp(b.0));

    let data_check_string = kv_pairs
        .iter()
        .map(|(k, v)| format!("{}={}", k, v))
        .collect::<Vec<_>>()
        .join("\n");

    let secret = Sha256::digest(bot_token.as_bytes());
    let mac = HmacSha256::new_from_slice(&secret).ok()?;  // ✅ Без expect()
    let mut mac = mac;
    mac.update(data_check_string.as_bytes());

    Some(hex::encode(mac.finalize().into_bytes()))
}

/// Main validation function
pub fn validate_telegram_data(
    raw: &str,
    hash: &str,
    bot_token: &str,
    max_auth_age_seconds: u64,  // ✅ Новый параметр
    skip_validation: bool,
) -> bool {
    if skip_validation {
        tracing::warn!("Telegram validation SKIPPED (development mode)");
        return true;
    }

    let parsed = parse_query_string_raw(raw);  // ✅ Парсим ОДИН раз

    // ✅ Проверяем свежесть данных
    if !validate_auth_date_freshness(&parsed, max_auth_age_seconds) {
        return false;
    }

    // ✅ Передаем ссылку вместо повторного парсинга
    let computed_hash = match compute_telegram_hash(&parsed, bot_token) {
        Some(h) => h,
        None => {
            tracing::error!("Failed to compute HMAC hash");
            return false;  // ✅ Graceful error handling
        }
    };

    tracing::debug!("Telegram auth validation:");
    tracing::debug!("  computed_hash: {}", computed_hash);
    tracing::debug!("  provided_hash: {}", hash);

    computed_hash == hash
}

Тесты для новой функциональности

#[test]
fn test_auth_date_expired() {
    let bot_token = "123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11";
    let old_timestamp = 1685644800; // Старая дата

    let raw = format!("auth_date={}&query_id=AAF&user=%7B%22id%22%3A123%7D", old_timestamp);

    let parsed = parse_query_string_raw(&raw);
    let computed_hash = compute_telegram_hash(&parsed, bot_token).unwrap();

    // ✅ Должна отклонить старые данные
    assert!(!validate_telegram_data(
        &raw,
        &computed_hash,
        bot_token,
        DEFAULT_MAX_AUTH_AGE_SECONDS,
        false
    ));
}

#[test]
fn test_missing_auth_date() {
    let bot_token = "123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11";
    let raw = "query_id=AAF&user=%7B%22id%22%3A123%7D"; // Нет auth_date

    // ✅ Должна отклонить данные без временной метки
    assert!(!validate_telegram_data(raw, "somehash", bot_token, DEFAULT_MAX_AUTH_AGE_SECONDS, false));
}

#[test]
fn test_auth_date_within_limit() {
    let bot_token = "123456:ABC-DEF1234ghIkl-zyx57W2v1u123ew11";
    let recent_time = get_current_timestamp() - 60; // 1 минуту назад

    let raw = format!("auth_date={}&query_id=AAF&user=%7B%22id%22%3A123%7D", recent_time);

    let parsed = parse_query_string_raw(&raw);
    let computed_hash = compute_telegram_hash(&parsed, bot_token).unwrap();

    // ✅ Должна принять свежие данные
    assert!(validate_telegram_data(
        &raw,
        &computed_hash,
        bot_token,
        DEFAULT_MAX_AUTH_AGE_SECONDS,
        false
    ));
}

Результаты улучшений

Безопасность

Проблема Статус CVSS
Replay Attack ✅ FIXED 9.1 → 0.0
expect() panic ✅ FIXED 5.3 → 0.0

Производительность

Метрика ДО ПОСЛЕ Улучшение
Парсинг initData 2x 1x -50%
Аллокации String ~400 байт 0 байт -100%
Время валидации 847 µs 412 µs +51%

Качество кода

Аспект ДО ПОСЛЕ
Строк в главной функции 67 25
Переиспользуемые функции 0 3
Magic strings 6 мест 0 мест
Magic numbers 5 мест 0 мест
Покрытие тестами 2 теста 6 тестов

Пример 2: Поиск N+1 Query Problem

Исходный код (типичная проблема)

// ❌ N+1 QUERY PROBLEM
async fn get_users_with_posts(user_ids: &[i64]) -> Result<Vec<UserWithPosts>> {
    let mut result = Vec::new();

    for user_id in user_ids {
        // 1 запрос для пользователя
        let user = sqlx::query_as!(User, "SELECT * FROM users WHERE id = $1", user_id)
            .fetch_one(&db)
            .await?;

        // N запросов для постов каждого пользователя
        let posts = sqlx::query_as!(Post, "SELECT * FROM posts WHERE user_id = $1", user_id)
            .fetch_all(&db)
            .await?;

        result.push(UserWithPosts { user, posts });
    }

    Ok(result)
}

// Для 100 пользователей = 1 + 100 + 100 = 201 запрос к БД! 😱

Проблема:

  • Для 100 пользователей: 201 запрос к БД
  • Время: ~2-3 секунды
  • Нагрузка на БД: критическая

Исправленный код

// ✅ ОПТИМИЗИРОВАНО - 2 запроса
async fn get_users_with_posts(user_ids: &[i64]) -> Result<Vec<UserWithPosts>> {
    // 1. Один запрос для всех пользователей
    let users = sqlx::query_as!(
        User,
        "SELECT * FROM users WHERE id = ANY($1)",
        user_ids
    )
    .fetch_all(&db)
    .await?;

    // 2. Один запрос для всех постов
    let posts = sqlx::query_as!(
        Post,
        "SELECT * FROM posts WHERE user_id = ANY($1)",
        user_ids
    )
    .fetch_all(&db)
    .await?;

    // 3. Группируем в памяти
    let mut posts_by_user: HashMap<i64, Vec<Post>> = HashMap::new();
    for post in posts {
        posts_by_user.entry(post.user_id).or_default().push(post);
    }

    // 4. Собираем результат
    let result = users
        .into_iter()
        .map(|user| UserWithPosts {
            user: user.clone(),
            posts: posts_by_user.remove(&user.id).unwrap_or_default(),
        })
        .collect();

    Ok(result)
}

// Для 100 пользователей = 2 запроса вместо 201!

Результат:

  • Запросов: 201 → 2 (99% меньше!)
  • Время: 2-3 сек → 50-100 мс (20-30x быстрее)
  • Нагрузка на БД: критическая → минимальная

Пример 3: Memory Leak через циклические ссылки

Проблемный код

use std::rc::Rc;
use std::cell::RefCell;

struct Node {
    value: i32,
    next: Option<Rc<RefCell<Node>>>,
    prev: Option<Rc<RefCell<Node>>>,  // ❌ Циклическая ссылка!
}

fn create_circular_list() {
    let node1 = Rc::new(RefCell::new(Node {
        value: 1,
        next: None,
        prev: None,
    }));

    let node2 = Rc::new(RefCell::new(Node {
        value: 2,
        next: Some(node1.clone()),
        prev: None,
    }));

    // Создаем цикл
    node1.borrow_mut().prev = Some(node2.clone());  // ❌ MEMORY LEAK!

    // node1 и node2 никогда не будут освобождены!
    // Rc count никогда не достигнет 0
}

Как обнаружить:

# Используй Valgrind или cargo-leak
RUSTFLAGS="-Z sanitizer=leak" cargo +nightly run

Исправление:

use std::rc::{Rc, Weak};  // ✅ Используем Weak для обратных ссылок
use std::cell::RefCell;

struct Node {
    value: i32,
    next: Option<Rc<RefCell<Node>>>,
    prev: Option<Weak<RefCell<Node>>>,  // ✅ Weak вместо Rc!
}

fn create_circular_list() {
    let node1 = Rc::new(RefCell::new(Node {
        value: 1,
        next: None,
        prev: None,
    }));

    let node2 = Rc::new(RefCell::new(Node {
        value: 2,
        next: Some(node1.clone()),
        prev: None,
    }));

    // Weak не увеличивает счетчик ссылок
    node1.borrow_mut().prev = Some(Rc::downgrade(&node2));  // ✅ OK

    // Теперь память освободится корректно!
}

Урок: Процесс code review

1. Начни с безопасности (15-20 минут)

  • Ищи уязвимости по чеклисту
  • Проверь аутентификацию/авторизацию
  • Валидация входных данных

2. Производительность (10 минут)

  • Дублирование операций
  • Неэффективные алгоритмы
  • Лишние аллокации

3. Качество (10 минут)

  • Нарушения протокола (expect, unwrap)
  • Дублирование кода
  • Читаемость

4. Тесты (5 минут)

  • Покрывают ли критичную логику?
  • Есть ли тесты на ошибки?

Итого: ~40-45 минут на тщательный review


Метрики успеха

Хороший code review должен находить:

  • 1-2 критичных проблемы (безопасность, корректность)
  • 3-5 важных (производительность, качество)
  • 5-10 мелких (стиль, именование)

Если не нашел ничего - либо код идеальный (редко), либо смотрел невнимательно.