Skip to content

Commit 6cc49af

Browse files
committed
Resolving comments
1 parent b4aad39 commit 6cc49af

File tree

1 file changed

+32
-82
lines changed

1 file changed

+32
-82
lines changed

mssql_python/helpers.py

Lines changed: 32 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -157,11 +157,9 @@ def sanitize_user_input(user_input: str, max_length: int = 50) -> str:
157157

158158
def validate_attribute_value(attribute, value, sanitize_logs=True, max_log_length=50):
159159
"""
160-
Validates attribute and value pairs for connection attributes and optionally
161-
sanitizes values for safe logging.
160+
Validates attribute and value pairs for connection attributes.
162161
163-
This function performs comprehensive validation of ODBC connection attributes
164-
and their values to ensure they are safe and valid before passing to the C++ layer.
162+
Performs basic type checking and validation of ODBC connection attributes.
165163
166164
Args:
167165
attribute (int): The connection attribute to validate (SQL_ATTR_*)
@@ -170,17 +168,8 @@ def validate_attribute_value(attribute, value, sanitize_logs=True, max_log_lengt
170168
max_log_length (int): Maximum length of sanitized output for logging
171169
172170
Returns:
173-
tuple: (is_valid, error_message, sanitized_attribute, sanitized_value) where:
174-
- is_valid is a boolean
175-
- error_message is None if valid, otherwise validation error message
176-
- sanitized_attribute is attribute as a string safe for logging
177-
- sanitized_value is value as a string safe for logging
178-
179-
Note:
180-
This validation acts as a security layer to prevent SQL injection, buffer
181-
overflows, and other attacks by validating all inputs before they reach C++ code.
171+
tuple: (is_valid, error_message, sanitized_attribute, sanitized_value)
182172
"""
183-
184173
# Sanitize a value for logging
185174
def _sanitize_for_logging(input_val, max_length=max_log_length):
186175
if not isinstance(input_val, str):
@@ -189,100 +178,61 @@ def _sanitize_for_logging(input_val, max_length=max_log_length):
189178
except:
190179
return "<non-string>"
191180

192-
# Remove control characters and non-printable characters
193-
# Allow alphanumeric, dash, underscore, and dot (common in encoding names)
181+
# Allow alphanumeric, dash, underscore, and dot
194182
sanitized = re.sub(r'[^\w\-\.]', '', input_val)
195183

196-
# Limit length to prevent log flooding
184+
# Limit length
197185
if len(sanitized) > max_length:
198186
sanitized = sanitized[:max_length] + "..."
199187

200-
# Return placeholder if nothing remains after sanitization
201188
return sanitized if sanitized else "<invalid>"
202189

203-
# Create sanitized versions for logging regardless of validation result
190+
# Create sanitized versions for logging
204191
sanitized_attr = _sanitize_for_logging(attribute) if sanitize_logs else str(attribute)
205192
sanitized_val = _sanitize_for_logging(value) if sanitize_logs else str(value)
206193

207-
# Attribute must be a non-negative integer
194+
# Basic attribute validation - must be an integer
208195
if not isinstance(attribute, int):
209196
return False, f"Attribute must be an integer, got {type(attribute).__name__}", sanitized_attr, sanitized_val
210197

211-
if attribute < 0:
212-
return False, f"Attribute value cannot be negative: {attribute}", sanitized_attr, sanitized_val
213-
214-
# Define attribute limits based on SQL specifications
215-
MAX_STRING_SIZE = 8192 # 8KB maximum for string values
216-
MAX_BINARY_SIZE = 32768 # 32KB maximum for binary data
217-
218-
# Attribute-specific validation
198+
# Define driver-level attributes that are supported
199+
SUPPORTED_ATTRIBUTES = [
200+
ConstantsDDBC.SQL_ATTR_ACCESS_MODE.value,
201+
ConstantsDDBC.SQL_ATTR_AUTOCOMMIT.value,
202+
ConstantsDDBC.SQL_ATTR_CONNECTION_TIMEOUT.value,
203+
ConstantsDDBC.SQL_ATTR_CURRENT_CATALOG.value,
204+
ConstantsDDBC.SQL_ATTR_LOGIN_TIMEOUT.value,
205+
ConstantsDDBC.SQL_ATTR_PACKET_SIZE.value,
206+
ConstantsDDBC.SQL_ATTR_TXN_ISOLATION.value
207+
]
208+
209+
# Check if attribute is supported
210+
if attribute not in SUPPORTED_ATTRIBUTES:
211+
return False, f"Unsupported attribute: {attribute}", sanitized_attr, sanitized_val
212+
213+
# Basic value type validation
219214
if isinstance(value, int):
220-
# General integer validation
221-
if value < 0 and attribute not in [
222-
# List of attributes that can accept negative values (very few)
223-
]:
215+
# For integer values, check if negative
216+
if value < 0:
224217
return False, f"Integer value cannot be negative: {value}", sanitized_attr, sanitized_val
225-
226-
# Attribute-specific integer validation
227-
if attribute == ConstantsDDBC.SQL_ATTR_CONNECTION_TIMEOUT.value:
228-
# Connection timeout has a maximum of UINT_MAX (4294967295)
229-
if value > 4294967295:
230-
return False, f"Connection timeout cannot exceed 4294967295: {value}", sanitized_attr, sanitized_val
231-
232-
elif attribute == ConstantsDDBC.SQL_ATTR_LOGIN_TIMEOUT.value:
233-
# Login timeout has a maximum of UINT_MAX (4294967295)
234-
if value > 4294967295:
235-
return False, f"Login timeout cannot exceed 4294967295: {value}", sanitized_attr, sanitized_val
236-
237-
elif attribute == ConstantsDDBC.SQL_ATTR_AUTOCOMMIT.value:
238-
# Autocommit can only be 0 or 1
239-
if value not in [0, 1]:
240-
return False, f"Autocommit value must be 0 or 1: {value}", sanitized_attr, sanitized_val
241-
242-
elif attribute == ConstantsDDBC.SQL_ATTR_TXN_ISOLATION.value:
243-
# Transaction isolation must be one of the predefined values
244-
valid_isolation_levels = [
245-
ConstantsDDBC.SQL_TXN_READ_UNCOMMITTED.value,
246-
ConstantsDDBC.SQL_TXN_READ_COMMITTED.value,
247-
ConstantsDDBC.SQL_TXN_REPEATABLE_READ.value,
248-
ConstantsDDBC.SQL_TXN_SERIALIZABLE.value
249-
]
250-
if value not in valid_isolation_levels:
251-
return False, f"Invalid transaction isolation level: {value}", sanitized_attr, sanitized_val
252218

253219
elif isinstance(value, str):
254-
# String validation
220+
# Basic string length check
221+
MAX_STRING_SIZE = 8192 # 8KB maximum
255222
if len(value) > MAX_STRING_SIZE:
256223
return False, f"String value too large: {len(value)} bytes (max {MAX_STRING_SIZE})", sanitized_attr, sanitized_val
257-
258-
# SQL injection pattern detection for strings
259-
sql_injection_patterns = [
260-
'--', ';', '/*', '*/', 'UNION', 'SELECT', 'INSERT', 'UPDATE',
261-
'DELETE', 'DROP', 'EXEC', 'EXECUTE', '@@', 'CHAR(', 'CAST('
262-
]
263-
264-
# Case-insensitive check for SQL injection patterns
265-
value_upper = value.upper()
266-
for pattern in sql_injection_patterns:
267-
if pattern.upper() in value_upper:
268-
return False, f"String value contains potentially unsafe SQL pattern: {pattern}", sanitized_attr, sanitized_val
269-
224+
270225
elif isinstance(value, (bytes, bytearray)):
271-
# Binary data validation
226+
# Basic binary length check
227+
MAX_BINARY_SIZE = 32768 # 32KB maximum
272228
if len(value) > MAX_BINARY_SIZE:
273229
return False, f"Binary value too large: {len(value)} bytes (max {MAX_BINARY_SIZE})", sanitized_attr, sanitized_val
274-
275-
# Check for suspicious binary patterns
276-
# Count null bytes (could indicate manipulation)
277-
null_count = value.count(0)
278-
# Too many nulls might indicate padding attack
279-
if null_count > len(value) // 4: # More than 25% nulls
280-
return False, "Binary data contains suspicious patterns", sanitized_attr, sanitized_val
281230

282231
else:
232+
# Reject unsupported value types
283233
return False, f"Unsupported attribute value type: {type(value).__name__}", sanitized_attr, sanitized_val
284234

285-
# If we got here, all validations passed
235+
# All basic validations passed
286236
return True, None, sanitized_attr, sanitized_val
287237

288238

0 commit comments

Comments
 (0)