Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 255 additions & 4 deletions SECURITY.md
Original file line number Diff line number Diff line change
@@ -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]