[SECURITY] SQL injection via string concatenation

[GAURAV-1313]

User input concatenated directly into SQL query.

[netlify]

✅ Deploy Preview for nofriction canceled.

Name	Link
🔨 Latest commit	1167e33
🔍 Latest deploy log	https://app.netlify.com/projects/nofriction/deploys/6a3c347b8142be000886c025

[GAURAV-1313]

AI Code Review

Summary

This PR introduces a new database access function with critical SQL injection vulnerability due to direct string interpolation of user input. It lacks input validation and proper query parameterization, presenting a significant security risk.

Key Findings

Bugs

• [CRITICAL] getUserByName — Direct string concatenation with unvalidated input 'name' leading to SQL injection vulnerability — allows attackers to manipulate the query and access or corrupt data.
• [MEDIUM] getUserByName — Missing validation of function parameter 'name' to confirm it is a valid, safe string — could cause errors or unintended queries if input format is unexpected.

Security

• CRITICAL SQL injection vulnerability in getUserByName function in src/db.js: user-controlled 'name' parameter is concatenated directly into the SQL query string without parameterization, allowing an attacker to execute arbitrary SQL commands.

Performance

• Potential SQL injection risk resulting in inefficient query plans because the 'name' parameter is concatenated directly into the query string rather than using parameterized queries, which can prevent the database from using indexes effectively.

Recommended Fixes

General

• Change getUserByName implementation to use parameterized queries/prepared statements to safely handle 'name' input and prevent SQL injection.
• Add input validation for 'name' parameter to reject null, undefined, or malformed input before querying the database.

Security Fixes

• SQL injection vulnerability in getUserByName function in src/db.js: user-controlled 'name' parameter is concatenated directly into the SQL query string without parameterization, allowing an attacker to execute arbitrary SQL commands. — Use parameterized queries or prepared statements to safely pass the 'name' parameter. For example, use db.execute('SELECT * FROM users WHERE name = ?', [name]) instead of string concatenation.

Performance Fixes

• Potential SQL injection risk resulting in inefficient query plans because the 'name' parameter is concatenated directly into the query string rather than using parameterized queries, which can prevent the database from using indexes effectively. — Use parameterized queries/prepared statements to safely inject variables and enable the database to leverage indexes for the 'name' field.

Suggested Tests

getUserByName

const db = { execute: jest.fn() };

// Mock the db module or object used in the src/db.js
jest.mock('../src/db', () => ({
  db
}), { virtual: true });

const { getUserByName } = require('../src/db');

describe('getUserByName', () => {
  beforeEach(() => {
    jest.clearAllMocks();
  });

  it('should return result of db.execute with correct query for a normal name', () => {
    db.execute.mockReturnValueOnce(Promise.resolve([{ id: 1, name: 'Alice' }]));
    const name = 'Alice';
    const expectedQuery = "SELECT * FROM users WHERE name = 'Alice'";
    const result = getUserByName(name);
    expect(db.execute).toHaveBeenCalledWith(expectedQuery);
    expect(result).toEqual(Promise.resolve([{ id: 1, name: 'Alice' }]));
  });

  it('should correctly form query with single-character name', () => {
    const name = 'A';
    const expectedQuery = "SELECT * FROM users WHERE name = 'A'";
    getUserByName(name);
    expect(db.execute).toHaveBeenCalledWith(expectedQuery);
  });

  it('should correctly form query with empty string name', () => {
    const name = '';
    const expectedQuery = "SELECT * FROM users WHERE name = ''";
    getUserByName(name);
    expect(db.execute).toHaveBeenCalledWith(expectedQuery);
  });

  it('should correctly handle name containing single quote characters', () => {
    const name = "O'Brien";
    const expectedQuery = "SELECT * FROM users WHERE name = 'O'Brien'";
    getUserByName(name);
    expect(db.execute).toHaveBeenCalledWith(expectedQuery);
  });

  it('should correctly handle name with SQL injection attempt', () => {
    const name = "Robert'; DROP TABLE users;--";
    const expectedQuery = "SELECT * FROM users WHERE name = 'Robert'; DROP TABLE users;--'";
    getUserByName(name);
    expect(db.execute).toHaveBeenCalledWith(expectedQuery);
  });

  it('should call db.execute even if name is null or undefined', () => {
    getUserByName(null);
    expect(db.execute).toHaveBeenCalledWith("SELECT * FROM users WHERE name = 'null'");

    getUserByName(undefined);
    expect(db.execute).toHaveBeenCalledWith("SELECT * FROM users WHERE name = 'undefined'");
  });

  it('should not modify the input argument', () => {
    const name = 'Alice';
    const nameCopy = name.slice();
    getUserByName(name);
    expect(name).toBe(nameCopy);
  });

  it('should return a Promise as returned by db.execute', () => {
    const mockReturn = Promise.resolve([{ id: 2, name: 'Bob' }]);
    db.execute.mockReturnValue(mockReturn);
    const result = getUserByName('Bob');
    expect(result).toBeInstanceOf(Promise);
    return expect(result).resolves.toEqual([{ id: 2, name: 'Bob' }]);
  });

  it('should throw if db.execute throws', () => {
    const error = new Error('DB failure');
    db.execute.mockImplementation(() => { throw error; });
    expect(() => getUserByName('test')).toThrow(error);
  });
});

Auto-Fix Available

Add the apply-ai-fixes label to this pull request to have RepoSpace
automatically generate and commit code fixes for the issues listed above.

---

4 issues found. Reviewed by RepoSpace AI.

[GAURAV-1313]

Auto-Fix Results

Applied (1)

• src/db.js — Fixed SQL injection vulnerability by using parameterized queries.

---

Generated by RepoSpace AI. Review AI commits before merging.