From 56a6504fb485777505377eb6e0249cdf9dd41160 Mon Sep 17 00:00:00 2001 From: Silapareddy Praveen Kumar Reddy <157562658+Silapareddy-Praveen-Kumar-Reddy@users.noreply.github.com> Date: Thu, 15 May 2025 09:22:19 -0600 Subject: [PATCH] Update SECURITY.md --- SECURITY.md | 259 +++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 255 insertions(+), 4 deletions(-) diff --git a/SECURITY.md b/SECURITY.md index 456fe0e..5d9f9e6 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -1,7 +1,258 @@ -Please use https://g.co/vulnz to report security vulnerabilities. +# Security Vulnerability Report -We use https://g.co/vulnz for our intake and triage. For valid issues we will do coordination and disclosure here on -GitHub (including using a GitHub Security Advisory when necessary). +## Summary +The `validate_user` function checks usernames for some conditions. Even though the function is coded with strict validation criteria, there exist vulnerability areas and improvement areas for increased security, efficiency, and maintainability. -The Google Security Team will process your report within a day, and respond within a week (although it will depend on the severity of your report). +--- +## Technical Details + +### 1. **Case Sensitivity** +- The approach is case-insensitive by passing the username to lower case. However, this might lead to security problems if the database also does not support case-insensitivity on a consistent basis. +- **Risk:** If the database stores usernames in mixed cases, there could be issues with authentication or duplication. + +### 2. **Validation Rules** +- The method employs a loop (`for char in username`) to scan bad characters. This method can be susceptible to bypasses via Unicode characters or non-ASCII input. +- **Risk:** Unicode homoglyph attacks (e.g., with "аdmin" using Cyrillic "а") can bypass validation. +- **Proposal:** Enforce a stricter regex pattern to check for usernames. + +### 3. **Reserved Words** +- The role prohibits certain reserved words (`admin`, `root`, etc.). Nevertheless, the reserved words are hardcoded and not complete. +- **Risk:** Users can register usernames such as reserved words, i.e., `adm1n` or `root123`. +- **Proposal:** Maintain a more extensive collection of reserved words and add fuzzy matching (e.g., Levenshtein distance). + +### 4. **Consecutive Dots** +- The functionality verifies consecutive dots with the substring `".." in username`. It works well but doesn't stop leading or trailing dots (e.g., `.user.`). +- **Risk:** Trailing/leading dots can be an issue for certain systems or APIs. +- **Recommendation:** Enhance validation to limit leading and trailing dots. + +### 5. **Error Messages** +- The method supplies useful error messages for an invalid username. +- **Risk:** These error messages can leak internal logic to attackers, enabling them to launch targeted attacks. +- **Recommendation:** Replace some of the error messages with generic ones (e.g., "Invalid username"). + +### 6. **Efficiency** +- The process of iteratively validating different conditions repeatedly can become inefficient in case of large numbers of users. +- **Risk:** Under malicious or excessive inputs, the function will be a performance bottleneck. +- **Recommendation:** Reduce the function by employing regex or a one-pass validation approach. + +### 7. **Regex Denial of Service (ReDoS)** +- Although regex is not utilized inside the function, the inclusion of regex to check can make it vulnerable to ReDoS attacks if not implemented with patterns correctly. +- **Tip:** Apply regex with well-designed patterns to prevent catastrophic backtracking. + +--- + +## Impact +- **Authentication Issues:** Case mismatches between the database and application can lead to incorrect authentication or duplicate accounts. +- **Validation Bypasses:** Unicode and homoglyph attacks can facilitate attacks through malicious usernames. +- **Information Leakage:** Detailed error messages will enable attackers to build usernames to circumvent validation. + +--- + +## Recommendations +1. **Regex Validation:** + - Employ a regex pattern instead of the character loop and conditions. + - Sample regex: `^(?![0-9])(?!.*\.\.)(?!.*[._]$)[a-zA-Z0-9._]{minlen}` + - Does not start with a digit. + - Forbids sequential dots. + - Restricts characters to letters, numbers, dots, and underscores. + +2. **Case Insensitivity:** + - Ensure that case insensitivity is enforced consistently across the application and database. + +3. **Reserved Words:** + - Maintain an ongoing and active list of reserved words in a distinct configuration file or database table. + - Use fuzzy matching to detect similar usernames. + +4. **Error Messages:** + - Replace detailed error messages with a generic one. For example: + ```python + return "Error: Invalid username." + ``` + +5. **Efficiency:** + - Sanitize usernames with a one-pass regex instead of in multiple conditions. + +6. **Testing:** + - Add test cases for Unicode characters, leading/trailing dots, and edge cases (e.g., very long usernames). + +--- + +## Affected Version +- Made available code snippet (no version information given). + +--- + +## Steps to Reproduce +1. Validation of testing with the provided usernames: + - `.admin` + - `.user.` + - `админ` (Cyrillic "а") + - `root123` + +2. Observe the validation result and error messages. + +--- + +## Suggested Fix +Here’s a potential fix using regex for username validation: + +```python name=validate_user_with_regex.py +import re + +def validate_user(username, minlen): + """Checks if the received username matches the required conditions.""" + if not isinstance(username, str): + return "Error: Invalid username." + if minlen < 1: + return "Error: Invalid username." + + # Regex for validation + pattern = r"^(?![0-9])(?!.*\.\.)(?!.*[._]$)[a-zA-Z0-9._]{" + str(minlen) + r",}$" + if not re.match(pattern, username.lower()): + return "Error: Invalid username." + + # Reserved words + reserved_words = {"admin", "root", "user", "test"} + if username.lower() in reserved_words: + return "Error: Invalid username." + + return True +``` + +--- + +## Updated Test Cases +Here is the updated test suite for thorough testing: + +```python name=test_validate_user.py +import unittest +from validate_user_with_regex import validate_user + +class TestValidateUser(unittest.TestCase): + + def test_valid_usernames(self): + # Valid usernames + self.assertTrue(validate_user("valid_user", 5)) + self.assertTrue(validate_user("user123", 5)) + self.assertTrue(validate_user("user.name", 5)) + self.assertTrue(validate_user("user_name", 3)) + + def test_minimum_length(self): + # Username shorter than minimum length + self.assertEqual( + validate_user("abc", 5), + "Error: Invalid username." + ) + + def test_case_insensitivity(self): + # Case-insensitivity + self.assertTrue(validate_user("ValidUser", 5)) + self.assertTrue(validate_user("VALIDUSER", 5)) + + def test_invalid_characters(self): + # Invalid characters + self.assertEqual( + validate_user("user@name", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("user!name", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("user#name", 5), + "Error: Invalid username." + ) + + def test_starting_with_digit(self): + # Username starting with a digit + self.assertEqual( + validate_user("1username", 5), + "Error: Invalid username." + ) + + def test_reserved_words(self): + # Reserved words + self.assertEqual( + validate_user("admin", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("root", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("user", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("test", 5), + "Error: Invalid username." + ) + + def test_unicode_and_homoglyphs(self): + # Unicode characters and homoglyph attacks + self.assertEqual( + validate_user("админ", 5), # Cyrillic "а" + "Error: Invalid username." + ) + self.assertEqual( + validate_user("usеr", 5), # Homoglyph "е" + "Error: Invalid username." + ) + + def test_consecutive_dots(self): + # Consecutive dots + self.assertEqual( + validate_user("user..name", 5), + "Error: Invalid username." + ) + + def test_leading_and_trailing_dots(self): + # Leading and trailing dots + self.assertEqual( + validate_user(".username", 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("username.", 5), + "Error: Invalid username." + ) + + def test_edge_cases(self): + # Edge cases + self.assertEqual( + validate_user("", 1), + "Error: Invalid username." + ) + self.assertEqual( + validate_user("a" * 256, 5), + "Error: Invalid username." + ) # Very long username + + def test_non_string_input(self): + # Non-string input + self.assertEqual( + validate_user(12345, 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user(None, 5), + "Error: Invalid username." + ) + self.assertEqual( + validate_user(["user"], 5), + "Error: Invalid username." + ) + +if __name__ == "__main__": + unittest.main() +``` + +--- + +Thank you for addressing this issue promptly. + +Sincerely, +[Your Name]