ch03-02-sql-injection-review
You've been asked to review a database query tool before it goes to production. The developer is new to security and made several classic mistakes. SQL injection vulnerabilities can lead to data breaches, data loss, and complete system compromise.
This exercise builds on your code review skills from Chapter 2, now with a security focus. SQL injection is consistently in the OWASP Top 10 - it's one of the most common and dangerous vulnerabilities in web applications.
Your task: Identify ALL security vulnerabilities, categorize them by severity, and propose secure alternatives using parameterized queries.
🎯 Learning Objectives
💬 Discussion
- How does SQL injection work? What allows it to happen?
- Why is checking for "DROP" and "DELETE" not sufficient protection?
- What's the fundamental problem with string concatenation in SQL?
- How do parameterized queries prevent injection?
💡 Hints
Hint 1
Look for string concatenation patterns like format!() or push_str() that include user input directly in SQL queries.
Hint 2
The blocklist approach (checking for "DROP" and "DELETE") can be bypassed. Consider: '; SELECT * FROM users WHERE role='admin' --
Hint 3
Issues to find:
- Name filter: SQL injection via string concatenation
- Email domain filter: SQL injection (no validation)
- Sort column: SQL injection (arbitrary column/expression)
- Sort order: Injection possible (only checks exact match)
- get_user: user_id is String, concatenated without validation
- update_nickname: Direct string concatenation
- Architecture: UPDATE tool on "read-only" server
⚠️ Try the exercise first! Show Solution
#![allow(unused)] fn main() { // Secure implementation of search_users using parameterized queries async fn search_users(pool: &DbPool, input: SearchUsersInput) -> anyhow::Result<Vec<User>> { let mut conditions = vec!["1=1".to_string()]; let mut params: Vec<String> = vec![];if let Some(name) = &input.name { conditions.push("name LIKE ?".to_string()); params.push(format!("%{}%", name)); } if let Some(domain) = &input.email_domain { conditions.push("email LIKE ?".to_string()); params.push(format!("%@{}", domain)); } // For ORDER BY, use an allowlist - can't parameterize column names let allowed_columns = ["id", "name", "email"]; let order_clause = match &input.sort_by { Some(col) if allowed_columns.contains(&col.as_str()) => { let direction = match &input.sort_order { Some(o) if o.to_lowercase() == "desc" => "DESC", _ => "ASC", }; format!(" ORDER BY {} {}", col, direction) } _ => String::new(), }; let query = format!( "SELECT id, name, email, role FROM users WHERE {} LIMIT 100{}", conditions.join(" AND "), order_clause ); // Build query with dynamic binding let mut query_builder = sqlx::query_as::<_, (i64, String, String, String)>(&query); for param in &params { query_builder = query_builder.bind(param); } let rows = query_builder.fetch_all(pool.as_ref()).await?; Ok(rows.into_iter().map(|(id, name, email, role)| { User { id, name, email, role } }).collect()) }}
// Key security principles: // - Never use string concatenation for SQL with user input // - Blocklists can always be bypassed - use allowlists instead // - Parameterized queries separate SQL structure from data // - Defense in depth: read-only connections, least privilege, audit logging // - Code comments don't enforce security - "read-only server" with UPDATE tool
🤔 Reflection
- Why can't you parameterize ORDER BY column names?
- What's the difference between escaping quotes and parameterized queries?
- If the database user only has SELECT permission, is SQL injection still dangerous?
- How would you test for SQL injection in an automated way?