False negative: NestJS TypeORM SQLInjection vulnerability not detected

[plbbowden1]

Description of the issue I am testing CodeQL on a simple NestJS test repo before bringing it into our enterprise CI/CD pipeline, and I am receiving a false negative from the SQLInjection query (CWE-089) in the javascript library.

Code samples quote.controller.ts

import { Body, Controller, Post } from '@nestjs/common';
import { QuoteService } from './quote.service';

@Controller('/api/quote')
export class QuoteController {
  constructor(private readonly quoteService: QuoteService) {}

  @Post('test')
  async test(@Body() sql: { rawSql: string }) {
    const { rawSql } = sql;
    return await this.quoteService.test(rawSql);
  }
}

quote.service.ts

import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';
import { Repository } from 'typeorm';
import { Quote } from './entities';

@Injectable()
export class QuoteService {
  constructor(
    @InjectRepository(Quote)
    private quoteRepository: Repository<Quote>,
  ) {}

  async test(sql: string) {
    console.log(sql);
    return await this.quoteRepository.query(sql);
  }
}

quote.entity.ts

import { Column, Entity, PrimaryGeneratedColumn } from 'typeorm';

@Entity()
export class Quote {
  @PrimaryGeneratedColumn()
  id?: number;

  @Column()
  quote?: string;

  @Column()
  character?: string;
}

dataSourceOptions (for TypeORM configuration)

import { DataSource, DataSourceOptions } from 'typeorm';

export const dataSourceOptions: DataSourceOptions = {
  type: 'sqlite',
  database: 'src/db/quotes.db',
  synchronize: true, // disable at production
  logging: true,
  entities: ['dist/**/*.entity{.ts,.js}'],
  migrations: ['dist/db/migrations/*{.ts,.js}'],
};

const dataSource = new DataSource(dataSourceOptions);

export default dataSource;

CodeQL Rule Rule ID: js/sql-injection

Environment typescript (5.1.3) NodeJS (18.17.0) NestJS (10.0.1) TypeORM (10.0.0)

Thank you for your time!

[rocketsciencenerd]

I'm having the same issue - not sure what's up here.

[MathiasVP]

Hi @plbbowden1,

Thanks for reporting this. I've forwarded this to our JS team, and they'll take a look at this soon 👍

[erik-krogh]

Thanks for the detailed report.

Yes, this is something our analysis doesn't cover.

Covering the specific FN you mention just requires adding the below:

class TypeORMSink extends SQL::SqlString {
  TypeORMSink() {
    this = API::Node::ofType("typeorm", "Repository").getMember("query").getParameter(0).asSink()
  }
}

A more complete model for TypeORM will require a lot more work, and I'm not sure when we'll have time for that.
I've added an internal issue about it.

We do have an experimental model for TypeORM, but that model doesn't cover your example.

[plbbowden1]

Thank you for your response, @erik-krogh! I'll go ahead and close out this issue.