fix: Replace SHA-1 with SHA-256 in getHash() (SDL compliance, CWE-327) - #140
Merged
Conversation
Agent-Logs-Url: https://github.com/microsoft/vscode-ext-kusto/sessions/dc481474-6c8e-463d-80bd-d5602c62a6cf Co-authored-by: tanmaya-panda1 <108695755+tanmaya-panda1@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix weak hashes by removing SHA-1 usage
fix: Replace SHA-1 with SHA-256 in getHash() (SDL compliance, CWE-327)
Mar 31, 2026
tanmaya-panda1
marked this pull request as ready for review
March 31, 2026 07:34
ag-ramachandran
approved these changes
Mar 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SHA-1 is cryptographically broken and banned by Microsoft SDL standards.
getHash()insrc/extension/utils.tsusedcreateHash('sha1')to generate AppInsights connection identifiers, triggering security finding SM04514 (js/weak-hashes).Change
src/extension/utils.ts— swap algorithm identifier fromsha1tosha256:export function getHash(value: string) { - return createHash('sha1').update(value).digest('hex'); + return createHash('sha256').update(value).digest('hex'); }Compatibility
The sole consumer (
appInsights/index.ts) uses the hash as an internal string key — stored in VS CodeMementoandSecretStorage, compared with===, used as aMapkey. No length constraints exist anywhere in the call chain (SHA-256 output is 64 hex chars vs. SHA-1's 40).Existing AppInsights connections stored with SHA-1-based IDs will become orphaned after this change; users will need to re-add them once. The existing error path (
'Failed to load secrets from saved information') handles this gracefully.Original prompt
Security Bug: [SM04514] Weak Hashes — SHA-1 usage violates Microsoft SDL standards
Vulnerability Details
js/weak-hashes(version: 0.1.470)src/extension/utils.ts, Line 136, Column 23-29aef3614bd9ef8b38fdcb864f481e72274e3c07b0Root Cause
The
getHash()function insrc/extension/utils.tsat line 136 usescreateHash('sha1')from Node.js'scryptomodule. SHA-1 is cryptographically broken and explicitly unapproved by Microsoft SDL standards.Impact Analysis — Where
getHash()is calledThe function is imported and used in exactly one consumer file:
File:
src/extension/kusto/connections/appInsights/index.ts(line 2 & 31)It hashes AppInsights credentials (
appId+appKey) to generate a connection identifier (idfield ofAppInsightsConnectionInfo).How the
idis used downstream (critical for regression analysis)AppInsightsConnectionInfo.id— Areadonly stringfield (defined insrc/extension/kusto/connections/types.tsline 12). Theidtype isstring— there are NO length constraints anywhere.storage.ts— Connection cache storage (src/extension/kusto/connections/storage.ts):updateConnectionCache()(line 60-86): Usesinfo.idas a Map key viaconnectionsToSave.set(item.id, item)andconnectionsToSave.delete(options.info.id)— works with any string length.getCachedConnections()(line 52): ReturnsIConnectionInfo[]from memento — no length constraints onid.storage.ts— Secret storage (src/extension/kusto/connections/storage.ts):getConnectionSecret(key: string)(line 41): CallssecretStorage.get(key)— VS CodeSecretStorageaccepts any string key.addConnectionSecret(key: string, secret: string)(line 45): CallssecretStorage.store(key, secret)— no length constraints.removeConnectionSecret(key: string)(line 48): CallssecretStorage.delete(key)— no length constraints.baseConnection.ts— Schema cache (src/extension/kusto/connections/baseConnection.ts):schemaCacheId(line 48-49): Usesthis.info.id.toLowerCase()— works with any string.getSchema()(line 54): Same pattern — no length constraints.appInsights/index.ts— Connection lookup (line 27):getCachedConnections().find((item) => item.id === info.appInsightsId)— exact string equality, works regardless of hash length.types.ts— Display info (line 36-48):getDisplayInfo()usesinfo.displayName || info.id— fallback display, works with any string.Regression Risk Assessment
idas a genericstringkey — no truncation, no fixed-length fields, no database columns.SecretStorageandMemento) will not match the new SHA-256 based ids. However, looking at the flow, theidis recomputed fromappId:appKeyeach timeconnectionInfofrom()is called. When a user reconnects, a new id will be generated. Old orphaned entries in storage will be harmless (never looked up again).getCachedConnections().find((item) => item.id === info.appInsightsId)lookup on line 27 only works if the id was previously saved with the same algorithm. After the fix, users will need to re-add AppInsights connections (a one-time action). This is an acceptable trade-off for a security fix.Required Fix
Replace
'sha1'with'sha256'in thegetHash()function insrc/extension/utils.ts:This is a single-line change. The
createHashAPI from Node.jscryptomodule supports'sha256'as a drop-in replacement — same interface, same return type (Hash), same.update().digest('hex')chain.Corner Cases Verified
stringtype, no fixed-width checks anywhereThis pull request was created from Copilot chat.