Skip to content

Mitigations for potential security risks in template parameter #779

@Yuan325

Description

@Yuan325

Prerequisites

What are you trying to do that currently feels hard or impossible?

The addition of templateParameters is valuable for Toolbox users. However, since template parameters can directly replace identifiers, column names, and table names, they are prone to SQL injections.

Do note that basic parameters are always preferred for performance and safety reasons.

Suggested Solution(s)

We are proposing two solutions to mitigate this risk.

(1) Adding an escape field to template parameter. If user uses templateParameter with this field, we will add double quotes (or backticks/square brackets depending on the db) to escape identifiers such as column names or table names.

tools:
 select_columns_from_table:
    kind: postgres-sql
    source: my-pg-instance
    statement: SELECT * FROM {{.tableName}};
    description: Use this tool to list all information from a specific table.
    templateParameters:
      - name: tableName
        type: string
        description: Table to select from
        escape: true

In the example above, the statement will resolve to SELECT * FROM "flight_table";. This method does reduces some risks but still prone to SQL injections.

(2) Adding a new allowedValues field in templateParameters that allows users to provide an allow-list. In the example below, tableName could be either flights_table or tickets_table. Any other values provided will cause an error and the query will NOT be performed.

tools:
 select_columns_from_table:
    kind: postgres-sql
    source: my-pg-instance
    statement: SELECT * FROM {{.tableName}}
    description: Use this tool to list all information from a specific table.
    templateParameters:
      - name: tableName
        type: string
        description: Table to select from
        allowedValues:
          - flights_table
          - tickets_table

Alternatives Considered

(2a) adding a new enum parameter type. Similar to allowedValues, we will instead add a new parameter type enum:

    templateParameters:
      - name: tableName
        type: enum
        description: Table to select from.
        enumType: string
        values:
          - flights_table
          - tickets_table

With this type, user can provide a list of values. There's also potential to support retrieving an allow-list from the description.

(3) Adding a flag --allow-unsafe-tools if user wants to use the templateParameters. Without the flag, users WILL NOT be able to use tools that have templateParameters.

(4) Adding a field for user to indicate runSingleStatement. The field will default to false. When specified, Toolbox will only grab the first statement (the statement before the first ;).

tools:
 select_columns_from_table:
    kind: postgres-sql
    source: my-pg-instance
    statement: SELECT * FROM {{.tableName}}
    runSingleStatement: true
    description: Use this tool to list all information from a specific table.

Additional Details

No response

Metadata

Metadata

Assignees

Labels

priority: p1Important issue which blocks shipping the next release. Will be fixed prior to next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
No fields configured for Feature.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions