Fix Get-DecryptedObject

[Philippe-Eversa]

Change LEN by DATALENGTH to avoid incorrect decryption when binary password or service master key values end with 0x20

Type of Change

• Bug fix (non-breaking change, fixes Copy-DbaLinkedServer copies very rarely incorrect password #10401)
• New feature (non-breaking change, adds functionality, fixes # )
• Breaking change (affects multiple commands or functionality, fixes # )
• Ran manual Pester test and has passed (Invoke-ManualPester)
• Adding code coverage to existing functionality
• Pester test is included
• If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
• Unit test is included
• Documentation
• Build system

Purpose

Correct a bug that almost never happen.
To make the bug happen, one of the folowing binary value must end with 0x20 :

pwdhash FROM master.sys.syslnklgns
imageval FROM sys.sysobjvalues WHERE valclass = 28 AND valnum = 2
crypt_property FROM sys.key_encryptions WHERE key_id = 102 AND thumbprint = 0x0300000001

These tables are only accessible with DAC connection

[andreasjordan]

@claude please review this pull request.

[claude]

Claude finished @andreasjordan's task in 1m 25s —— View job

---

PR Review

• Read changed file and diff
• Analyze the fix correctness
• Check code style and conventions
• Post review findings

---

Summary

This PR fixes a subtle but real decryption bug in Get-DecryptedObject.ps1. The root cause is that SQL Server's LEN() function strips trailing 0x20 bytes (space character) from binary columns before returning the length, while DATALENGTH() returns the true byte count. When the encrypted binary data in pwdhash, imageval, or crypt_property ends with 0x20, LEN() underreports the length, causing SUBSTRING() to extract an incomplete ciphertext, which then produces an incorrect decryption result.

---

Fix Analysis

The four changed SUBSTRING calls are all in the same pattern:

-- BEFORE (wrong for binary ending in 0x20)
SUBSTRING(column, start, LEN(column) - n)

-- AFTER (correct - uses actual byte length)
SUBSTRING(column, start, DATALENGTH(column) - n)

The four fixed locations:

Line	Column	Context
55	crypt_property	SMK query (primary thumbprint)
58	crypt_property	SMK query (fallback thumbprint)
109	syslnklgns.pwdhash	LinkedServer password extraction
122	obj.imageval	Credential password extraction

The fix is correct. DATALENGTH() is the appropriate function for binary columns when you need the true byte count.

---

Minor Observation

Line 115 still uses LEN:

WHERE LEN(syslnklgns.pwdhash) > 0

This is a filter predicate (exclude empty/NULL rows), not a SUBSTRING length, so it does not contribute to the decryption bug. In theory, if pwdhash were exactly 0x20, LEN would return 0 and the row would be incorrectly skipped — but a single-byte password hash is not a valid encrypted value for any scheme in use here, making the practical impact nil.

For consistency, this could be updated to DATALENGTH(syslnklgns.pwdhash) > 0, but it is not required to fix issue #10401. Fix this →

---

Code Style / CLAUDE.md Compliance

• No backticks used for line continuation ✅
• Comments preserved exactly ✅
• Double quotes used for strings ✅
• No new parameters introduced, so no attribute syntax concerns ✅

---

Verdict

Approve. The fix is technically sound, well-scoped, and correctly targets the exact cause of the bug. The author's explanation in the PR description is accurate — LEN silently truncates the SUBSTRING length for binary values ending in 0x20, and DATALENGTH resolves that. The change is minimal and safe (no behavioral change when values don't end in 0x20).

[andreasjordan]

Thanks for reporting this bug and providing a fix.

[potatoqualitee]

Very nice bug fix, thank you again 🙏🏼