Skip to content

Fix missing WHERE clause in DWSql upsert UPDATE branch#3698

Open
naxing123 wants to merge 1 commit into
mainfrom
msrc_fix
Open

Fix missing WHERE clause in DWSql upsert UPDATE branch#3698
naxing123 wants to merge 1 commit into
mainfrom
msrc_fix

Conversation

@naxing123

Copy link
Copy Markdown
Contributor

The dwsql (Azure Synapse) upsert builder emitted an UPDATE with no WHERE clause, so a single PUT upsert hitting the existing-row UPDATE branch overwrote every row in the target table and dropped the update DB policy (CWE-862).

Append WHERE {pkPredicates AND update DB policy} to the UPDATE branch, mirroring MsSqlQueryBuilder.Build(SqlUpsertQueryStructure) and the DWSql PATCH path. Add a regression test asserting the generated dwsql upsert UPDATE branch is scoped by the PK and the update database policy.

Why make this change?

Fixes a security defect (CWE-862, missing row scoping / authorization) in the Azure Synapse Analytics (dwsql / Dedicated SQL Pool) upsert query builder.
The dwsql upsert builder computed pkPredicates but never appended a WHERE clause to the generated UPDATE. As a result, any authenticated user with entity-level update permission could issue a single [PUT /api//] that hit the existing-row UPDATE branch and overwrite every row in the target table, while also silently discarding the row-level update database policy. The API still returns a normal single-record 200, so the mass overwrite is indistinguishable from a legitimate single-row update in application logs.
Not affected: the MSSQL/PostgreSQL/MySQL upsert builders and DAB's own DWSql PATCH path ([Build(SqlUpdateStructure)]), which all correctly scope the UPDATE with WHERE.

What is this change?

  • Summary of how your changes work to give reviewers context of your intent.
    Append WHERE {pkPredicates AND update DB policy} to the UPDATE branch, mirroring MsSqlQueryBuilder.Build(SqlUpsertQueryStructure) and the DWSql PATCH path. Add a regression test asserting the generated dwsql upsert UPDATE branch is scoped by the PK and the update database policy.

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

Given a dwsql entity Book (PK id) with update permission granted, the following existing-row upsert previously overwrote all rows:
PUT /api/Book/id/7
Content-Type: application/json

{
"title": "The Hobbit Returns to The Shire",
"publisher_id": 1234
}

Generated UPDATE branch before the fix (overwrites every row):
IF @ROWS_TO_UPDATE = 1 BEGIN UPDATE [dbo].[books] SET [title] = @param1, [publisher_id] = @Param2 END

Generated UPDATE branch after the fix (scoped to the target row and update policy):
IF @ROWS_TO_UPDATE = 1 BEGIN UPDATE [dbo].[books] SET [title] = @param1, [publisher_id] = @Param2 WHERE [dbo].[books].[id] = @param0 END

The dwsql (Azure Synapse) upsert builder emitted an UPDATE with no WHERE clause, so a single PUT upsert hitting the existing-row UPDATE branch overwrote every row in the target table and dropped the update DB policy (CWE-862).

Append WHERE {pkPredicates AND update DB policy} to the UPDATE branch, mirroring MsSqlQueryBuilder.Build(SqlUpsertQueryStructure) and the DWSql PATCH path. Add a regression test asserting the generated dwsql upsert UPDATE branch is scoped by the PK and the update database policy.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a critical security defect in the DWSQL (Azure Synapse) upsert query builder where the UPDATE branch could be emitted without a WHERE clause, potentially overwriting all rows and bypassing update DB policy scoping.

Changes:

  • Scope the DWSQL upsert UPDATE branch using WHERE {pkPredicates AND updateDbPolicy} (mirrors the MsSql upsert builder behavior).
  • Add a unit regression test asserting the DWSQL upsert UPDATE branch includes WHERE, the PK predicate, and the update DB policy predicate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
src/Core/Resolvers/DWSqlQueryBuilder.cs Adds WHERE scoping to the DWSQL upsert UPDATE branch using PK + update DB policy predicates.
src/Service.Tests/UnitTests/DwSqlQueryBuilderUpsertTests.cs Adds a regression unit test to ensure DWSQL upsert UPDATE is always scoped by PK and update DB policy.

Comment on lines +4 to +8
using System;
using System.Collections.Generic;
using System.Data;
using Azure.DataApiBuilder.Auth;
using Azure.DataApiBuilder.Config.DatabasePrimitives;
Comment on lines +130 to +134
string outColumn;
metadataProvider.Setup(x => x.TryGetBackingColumn(It.IsAny<string>(), It.IsAny<string>(), out outColumn))
.Callback(new TryGetColumnCallback((string entity, string field, out string? column)
=> _columnMapping.TryGetValue(field, out column)))
.Returns((string entity, string field, string column) => _columnMapping.ContainsKey(field));
Comment on lines +136 to +140
string outExposed;
metadataProvider.Setup(x => x.TryGetExposedColumnName(It.IsAny<string>(), It.IsAny<string>(), out outExposed))
.Callback(new TryGetColumnCallback((string entity, string field, out string? column)
=> _columnMapping.TryGetValue(field, out column)))
.Returns((string entity, string field, string column) => _columnMapping.ContainsKey(field));
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants