Skip to content

fix(php): Escape $ in double-quoted string literals to prevent code injection#7863

Merged
peombwa merged 2 commits into
mainfrom
peombwa/fix-php-dollar-injection
Jun 25, 2026
Merged

fix(php): Escape $ in double-quoted string literals to prevent code injection#7863
peombwa merged 2 commits into
mainfrom
peombwa/fix-php-dollar-injection

Conversation

@peombwa

@peombwa peombwa commented Jun 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Kiota's PHP writer emits schema-derived strings into PHP double-quoted literals using SanitizeDoubleQuote(), which escapes ", \, and control characters but not $. Because PHP evaluates $var, ${...}, and {$...} inside double-quoted strings, an attacker-controlled OpenAPI description (e.g. a media type like application/json${system('whoami && id')}) is emitted verbatim into the generated SDK source and executes arbitrary PHP/OS commands when the generated client runs.

This is a regression/coverage gap in the literal-injection hardening pass (#7603), which already added the equivalent $ escape for the Dart writer (Dart interpolates $ the same way). The PHP surface was missed.

Root cause

SanitizeForQuotedLiteral in StringExtensions.cs is language-agnostic and has no $ case, so $ falls through unescaped. The 2024 change that switched PHP enum values from single- to double-quotes (d472aeb2e) widened the exposure.

Fix

Add a PHP-specific SanitizePhpDoubleQuoteLiteral on PhpConventionService that runs the shared SanitizeDoubleQuote() and then escapes $\$, and route every PHP double-quoted emission site through it. This mirrors the existing SanitizeDartDoubleQuoteLiteral on DartConventionService exactly (same name pattern, signature, placement, and implementation), satisfying the project's cross-language consistency rule.

internal static string SanitizePhpDoubleQuoteLiteral(string? value) =>
    string.IsNullOrEmpty(value) ? string.Empty
        : value.SanitizeDoubleQuote().Replace("$", "\\$", StringComparison.Ordinal);

peombwa and others added 2 commits June 25, 2026 15:36
…njection (MSRC 123931)

PHP interpolates $var, ${...} and {$...} inside double-quoted strings. The shared SanitizeDoubleQuote helper does not escape $, so attacker-controlled OpenAPI input (descriptions, enum values, content types, discriminator/query names) emitted into PHP double-quoted literals could inject and execute arbitrary PHP/OS commands when the generated client runs.

Add SanitizePhpDoubleQuoteLiteral on PhpConventionService (mirroring the existing SanitizeDartDoubleQuoteLiteral on DartConventionService) that additionally escapes $, and use it at all PHP double-quote emission sites. Add a regression test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@peombwa
peombwa requested a review from Copilot June 25, 2026 23:10
@peombwa peombwa self-assigned this Jun 25, 2026
@msgraph-bot msgraph-bot Bot added this to Kiota Jun 25, 2026

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

This PR hardens Kiota’s PHP code generator against generated-source injection / unintended PHP variable interpolation by ensuring schema-derived values emitted into PHP double-quoted string literals also escape $ (in addition to the existing SanitizeDoubleQuote() behavior).

Changes:

  • Added PhpConventionService.SanitizePhpDoubleQuoteLiteral to escape $ after SanitizeDoubleQuote().
  • Routed PHP writer emission sites that use double-quoted literals (enum wire values, request content-type, accept header, discriminator property name, and query-parameter annotation) through the new sanitizer.
  • Added a regression test ensuring $ in enum wire values is emitted as \$ and not ${...}.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/Kiota.Builder.Tests/Writers/Php/CodeEnumWriterTests.cs Adds regression coverage to ensure $ is escaped in generated PHP enum constants.
src/Kiota.Builder/Writers/Php/PhpConventionService.cs Introduces PHP-specific double-quote literal sanitizer that escapes $ to prevent interpolation.
src/Kiota.Builder/Writers/Php/CodePropertyWriter.cs Uses the new sanitizer for query-parameter annotation string content.
src/Kiota.Builder/Writers/Php/CodeMethodWriter.cs Uses the new sanitizer for request body content-type, Accept header value, and discriminator property name string literals.
src/Kiota.Builder/Writers/Php/CodeEnumWriter.cs Uses the new sanitizer when emitting enum wire values into double-quoted PHP literals.

@peombwa
peombwa marked this pull request as ready for review June 25, 2026 23:14
@peombwa
peombwa requested a review from a team as a code owner June 25, 2026 23:14
@github-code-quality

Copy link
Copy Markdown

Code Coverage Overview

Languages: C#

C# / code-coverage/dotnet

The overall coverage in the branch is 71%. Coverage data for the branch is not yet available.

Show a code coverage summary of the most covered files.
File c93a400 +/-
/home/runner/wo...guageRefiner.cs 98%
/home/runner/wo...criptRefiner.cs 98%
/home/runner/wo...MethodWriter.cs 97%
/home/runner/wo...MethodWriter.cs 96%
/home/runner/wo...MethodWriter.cs 96%
/home/runner/wo...MethodWriter.cs 95%
/home/runner/wo...rs/GoRefiner.cs 93%
/home/runner/wo...KiotaBuilder.cs 90%
/home/runner/wo...ationService.cs 89%
/home/runner/wo...xGenerator.g.cs 72%

Code Coverage is in Public Preview. Learn more and provide us with your feedback.

@peombwa
peombwa enabled auto-merge (squash) June 25, 2026 23:16
@github-project-automation github-project-automation Bot moved this to In Progress 🚧 in Kiota Jun 25, 2026
@peombwa
peombwa merged commit 5e2a211 into main Jun 25, 2026
313 checks passed
@peombwa
peombwa deleted the peombwa/fix-php-dollar-injection branch June 25, 2026 23:44
@github-project-automation github-project-automation Bot moved this from In Progress 🚧 to Done ✔️ in Kiota Jun 25, 2026
GuillaumeBodson pushed a commit to GuillaumeBodson/Filer that referenced this pull request Jul 10, 2026
Updated
[Microsoft.AspNetCore.Components.Authorization](https://github.com/dotnet/dotnet)
from 10.0.8 to 10.0.9.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Components.Authorization's
releases](https://github.com/dotnet/dotnet/releases)._

No release notes found for this version range.

Commits viewable in [compare
view](https://github.com/dotnet/dotnet/commits).
</details>

Updated
[Microsoft.AspNetCore.Components.Web](https://github.com/dotnet/dotnet)
from 10.0.8 to 10.0.9.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Components.Web's
releases](https://github.com/dotnet/dotnet/releases)._

No release notes found for this version range.

Commits viewable in [compare
view](https://github.com/dotnet/dotnet/commits).
</details>

Updated
[Microsoft.AspNetCore.Components.WebAssembly](https://github.com/dotnet/dotnet)
from 10.0.8 to 10.0.9.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Components.WebAssembly's
releases](https://github.com/dotnet/dotnet/releases)._

No release notes found for this version range.

Commits viewable in [compare
view](https://github.com/dotnet/dotnet/commits).
</details>

Updated
[Microsoft.AspNetCore.Mvc.Testing](https://github.com/dotnet/dotnet)
from 10.0.8 to 10.0.9.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Mvc.Testing's
releases](https://github.com/dotnet/dotnet/releases)._

No release notes found for this version range.

Commits viewable in [compare
view](https://github.com/dotnet/dotnet/commits).
</details>

Updated [Microsoft.AspNetCore.OpenApi](https://github.com/dotnet/dotnet)
from 10.0.8 to 10.0.9.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.OpenApi's
releases](https://github.com/dotnet/dotnet/releases)._

No release notes found for this version range.

Commits viewable in [compare
view](https://github.com/dotnet/dotnet/commits).
</details>

Updated [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest)
from 18.6.0 to 18.7.0.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.NET.Test.Sdk's
releases](https://github.com/microsoft/vstest/releases)._

## 18.7.0

## What's Changed
* Add ARM64 msdia140.dll support to test platform packages by
@​jamesmcroft in microsoft/vstest#15689
* Update System.Memory from 4.5.5 to 4.6.3 by @​nohwnd in
microsoft/vstest#15706

## New Contributors
* @​jamesmcroft made their first contribution in
microsoft/vstest#15689

**Full Changelog**:
microsoft/vstest@v18.6.0...v18.7.0

Commits viewable in [compare
view](microsoft/vstest@v18.6.0...v18.7.0).
</details>

Updated [microsoft.openapi.kiota](https://github.com/microsoft/kiota)
from 1.32.2 to 1.32.5.

<details>
<summary>Release notes</summary>

_Sourced from [microsoft.openapi.kiota's
releases](https://github.com/microsoft/kiota/releases)._

## 1.32.5

## Added

- Added the `--allowed-external-origins` parameter to commands that load
OpenAPI descriptions. External references are now not loaded by default
to improve security; use this parameter to allow `*`, full URIs, URI
patterns, full paths, relative paths, or path patterns.
- Added support for the `isNonConsequential` confirmation property in
the AI capabilities extension (`x-ai-capabilities`) and mapped it to the
generated plugin manifest confirmation object (plugin manifest 2.4).
[#​7857](microsoft/kiota#7857)
- Added a DOM-surface regression integration test that diffs the kiota
public API export between the published NuGet generator and the current
changeset to catch breaking changes in generated SDKs.
[#​7858](microsoft/kiota#7858)

### Changed

- Rejected unsafe `static_template.file` references in generated plugin
manifests, preventing path traversal outside the manifest package.
- Workspace client and plugin generation now rejects workspace
configuration entries whose `outputPath` is rooted or escapes the
workspace.
- Sanitized client class and namespace names loaded from settings or the
`x-ms-kiota-info` OpenAPI extension before using them in generated code
and file paths.
- Removed support for specifying dependency install commands through the
`x-ms-kiota-info` OpenAPI description extension.


## 1.32.4

## Added

### Changed

- Fixed a code injection vulnerability in PHP generation by escaping `$`
in double-quoted string literals.
[#​7863](microsoft/kiota#7863)


## 1.32.3

## Added

### Changed

- Fixed a generated-source code injection vulnerability (CWE-94) in the
C# emitter where newlines in an OpenAPI `externalDocs.description` could
break out of the `///` doc comment and inject arbitrary code.
[#​7831](microsoft/kiota#7831)
- Fixed a bug that generares an empty model when allOf inheritance
schema is reached via a composed type
[#​7791](microsoft/kiota#7791)
- Fixed a bug in PHP generation where `parent::__construct()` was
generated for parent classes without a constructor
[#​7809](microsoft/kiota#7809)
[#​7810](microsoft/kiota#7810)


Commits viewable in [compare
view](microsoft/kiota@v1.32.2...v1.32.5).
</details>

Updated
[Testcontainers.PostgreSql](https://github.com/testcontainers/testcontainers-dotnet)
from 4.12.0 to 4.13.0.

<details>
<summary>Release notes</summary>

_Sourced from [Testcontainers.PostgreSql's
releases](https://github.com/testcontainers/testcontainers-dotnet/releases)._

## 4.13.0

# What's Changed

Thank you to everyone who contributed and shared their feedback 🤜🤛.

The NuGet packages for this release have been attested for supply chain
security using [`actions/attest`](https://github.com/actions/attest).
This confirms the integrity and provenance of the artifacts and helps
ensure they can be trusted:
[#​33686956](https://github.com/testcontainers/testcontainers-dotnet/attestations/33686956).

## 🚀 Features

* feat: Add Aspire dashboard module (#​1194) @​NikiforovAll
* feat: Add image name substitution hook (#​1710) @​HofmeisterAn
* feat(CosmosDb): Add get method AccountEndpoint (#​1707) @​srollinet
* feat: Improve image build failure messages (#​1700) @​HofmeisterAn

## 🐛 Bug Fixes

* fix: Restore tar archive write performance regressed by padding trim
(#​1719) @​HofmeisterAn
* chore: Bump Docker.DotNet from 4.3.2 to 4.3.3 (#​1714) @​HofmeisterAn
* chore(AspireDashboard): Cover connection string provider (#​1713)
@​HofmeisterAn

## 📖 Documentation

* docs: Add missing TC languages and reorder docs navigation (#​1711)
@​mdelapenya
* docs: Add note about unsupported BuildKit Dockerfile features (#​1696)
@​HofmeisterAn
* docs: Explain immutable builder behavior (#​1693) @​HofmeisterAn

## 🧹 Housekeeping

* chore: Enable Dependabot cooldown (#​1716) @​HofmeisterAn
* chore: Add nuget.config (#​1715) @​Rob-Hague
* chore(AspireDashboard): Cover connection string provider (#​1713)
@​HofmeisterAn
* chore: Bump Docker.DotNet from 4.2.0 to 4.3.2 (#​1712) @​HofmeisterAn
* chore: Bump sshd-docker image from 1.3.0 to 1.4.0 (#​1709)
@​HofmeisterAn
* chore: Rename runtime label and add buildkit and stale labels (#​1703)
@​HofmeisterAn
* fix: Guard expensive argument evaluation when logging (#​1702)
@​HofmeisterAn
* chore: Defer container ID truncation in logging (#​1701)
@​HofmeisterAn
* chore: Migrate to LoggerMessageAttribute (#​1697) @​HofmeisterAn

## 📦 Dependency Updates

* chore(deps): Bump the actions group with 2 updates (#​1721)
@[dependabot[bot]](https://github.com/apps/dependabot)
* chore(deps): Bump the actions group with 7 updates (#​1717)
@[dependabot[bot]](https://github.com/apps/dependabot)
* chore: Bump Docker.DotNet from 4.3.2 to 4.3.3 (#​1714) @​HofmeisterAn
* chore: Bump Docker.DotNet from 4.2.0 to 4.3.2 (#​1712) @​HofmeisterAn
* chore(deps): Bump the actions group with 4 updates (#​1698)
@[dependabot[bot]](https://github.com/apps/dependabot)


Commits viewable in [compare
view](testcontainers/testcontainers-dotnet@4.12.0...4.13.0).
</details>

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore <dependency name> major version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's major version (unless you unignore this specific
dependency's major version or upgrade to it yourself)
- `@dependabot ignore <dependency name> minor version` will close this
group update PR and stop Dependabot creating any more for the specific
dependency's minor version (unless you unignore this specific
dependency's minor version or upgrade to it yourself)
- `@dependabot ignore <dependency name>` will close this group update PR
and stop Dependabot creating any more for the specific dependency
(unless you unignore this specific dependency or upgrade to it yourself)
- `@dependabot unignore <dependency name>` will remove all of the ignore
conditions of the specified dependency
- `@dependabot unignore <dependency name> <ignore condition>` will
remove the ignore condition of the specified dependency and ignore
conditions


</details>

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done ✔️

Development

Successfully merging this pull request may close these issues.

3 participants