Skip to content

FabricAdapter.quote() does not escape closing brackets — invalid SQL and identifier-injection risk #399

@sdebruyn

Description

@sdebruyn

Summary

FabricAdapter.quote() wraps an identifier in [...] without escaping any ] characters inside it. T-SQL bracket quoting requires every literal ] to be doubled to ]] to avoid prematurely closing the bracket. Two consequences:

  1. Any identifier containing ] produces invalid T-SQL — the bracket is closed early and the parser sees garbage after it.
  2. A crafted identifier of the form foo];<arbitrary T-SQL>-- will terminate the bracket and inject SQL into whatever statement the macro is composing. The attack surface in a typical dbt project is small (model and column names are usually static), but it is not zero — projects that derive identifiers from a multi-tenant catalogue or otherwise pull names from external systems are exposed.

The same unescaped pattern is present in FabricColumn.quoted, FabricRelation.quoted, and five Jinja macro sites that compose bracket-quoted identifiers by hand.

Evidence (HEAD 0de2190, v1.10.0)

dbt/adapters/fabric/fabric_adapter.py#L36-L38:

@classmethod
def quote(cls, identifier):
    return "[{}]".format(identifier)

dbt/adapters/fabric/fabric_column.py#L8-L10:

@property
def quoted(self) -> str:
    return "[{}]".format(self.column)

dbt/adapters/fabric/fabric_relation.py#L19-L20:

def quoted(self, identifier):
    return "[{}]".format(identifier)

Macro sites with the same pattern:

Reproduction

{{ adapter.quote("weird]name") }}
{# emits [weird]name]  →  T-SQL parser sees [weird] followed by `name]` (syntax error) #}

{{ adapter.quote("foo];drop table bar--") }}
{# emits [foo];drop table bar--]  →  bracket closed, second statement appended #}

Suggested fix

Escape ] as ]] everywhere a bracket-quoted identifier is composed:

@classmethod
def quote(cls, identifier):
    return "[{}]".format(identifier.replace("]", "]]"))

Same change in FabricColumn.quoted, FabricRelation.quoted, and the five macro sites listed above (Jinja: | replace(']', ']]')).

A PR with the full change across all eight sites is linked from this issue.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions