Implement vulnerability remediation for Issue #86 and ICM ticket#2
Implement vulnerability remediation for Issue #86 and ICM ticket#2jorgelevy merged 2 commits intojorgelevy:mainfrom
Conversation
implementing vulnerability remediation for Issue #86 and ICM ticket. Both vulnerabilities have been addressed.
There was a problem hiding this comment.
Pull request overview
This PR aims to remediate a vulnerability (Issue Azure-Samples#86 / ICM) in the PromptFlow demo by removing unsafe input parsing and reducing SQL injection risk in the customer lookup tool.
Changes:
- Replaced
eval(inputs)withjson.loads(inputs)for string inputs. - Switched from string-interpolated SQL to a parameterized
pyodbcquery. - Refactored name fields into local variables before executing the query.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if inputs_dict['MiddleName'] == "": | ||
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName='{inputs_dict['FirstName']}' and MiddleName is NULL and LastName='{inputs_dict['LastName']}'""" | ||
| middle_name = "NULL" | ||
| else: | ||
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName='{inputs_dict['FirstName']}' and MiddleName='{inputs_dict['MiddleName']}' and LastName='{inputs_dict['LastName']}'""" | ||
| middle_name = inputs_dict['MiddleName'] | ||
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName=? and MiddleName=? and LastName=?""" |
There was a problem hiding this comment.
Null middle-name handling is now broken: when MiddleName is empty you set middle_name = "NULL" and still run MiddleName = ?. In SQL Server, MiddleName = NULL never matches (and the literal string "NULL" is not SQL NULL), so customers with a NULL MiddleName will not be returned. Suggest using None for the parameter and switching the WHERE clause to MiddleName IS NULL for that branch (or using a predicate that correctly matches NULLs).
| else: | ||
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName='{inputs_dict['FirstName']}' and MiddleName='{inputs_dict['MiddleName']}' and LastName='{inputs_dict['LastName']}'""" | ||
| middle_name = inputs_dict['MiddleName'] | ||
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName=? and MiddleName=? and LastName=?""" |
There was a problem hiding this comment.
The query still uses select * on SalesLT.Customer, and the flow sample output includes sensitive fields like PasswordHash and PasswordSalt. Returning these values (especially into an LLM-driven flow) is a data-exposure risk. Please select only the needed columns and explicitly exclude credential-related fields.
| sqlQuery = f"""select * from [SalesLT].[Customer] WHERE FirstName=? and MiddleName=? and LastName=?""" | |
| sqlQuery = """ | |
| SELECT | |
| CustomerID, | |
| NameStyle, | |
| Title, | |
| FirstName, | |
| MiddleName, | |
| LastName, | |
| Suffix, | |
| CompanyName, | |
| SalesPerson, | |
| EmailAddress, | |
| Phone, | |
| rowguid, | |
| ModifiedDate | |
| FROM [SalesLT].[Customer] | |
| WHERE FirstName=? AND MiddleName=? AND LastName=? | |
| """ |
No description provided.