Reviewed all library files and Mongoose models. Found 30 issues across security, correctness, performance, and code quality. The most critical issues were a hardcoded JWT secret fallback (now fixed), regex injection in Animal.findByName (now fixed), and a race condition in SiteStats.recordVisit (now fixed).
Severity Legend:
- CRITICAL: Exploitable security vulnerability or data-loss bug
- HIGH: Significant bug or security concern
- MEDIUM: Non-trivial issue that should be fixed
- LOW: Code quality or minor concern
Was: const JWT_SECRET = process.env.JWT_SECRET || 'animal-stats-secret-key-change-in-production';
If JWT_SECRET was not set in the environment, the application silently fell back to a publicly-visible secret baked into source code. Any attacker who reads the source could forge arbitrary JWT tokens. The same hardcoded fallback was duplicated in api/auth.js line 25.
Fix applied: Removed the fallback — now throws at startup if JWT_SECRET is missing, matching how mongodb.js handles MONGODB_URI. Also deduplicated: api/auth.js now imports JWT_SECRET from lib/auth.js.
JWT_SECRET is exported in module.exports, which means any module that imports lib/auth.js gets access to sign tokens, not just verify them. This violates the principle of least privilege. Only verification helpers should be exported; signing should be done through a dedicated createToken() function.
verifyToken returns null for all errors (expired, malformed, wrong algorithm) without distinguishing them. Callers cannot tell the difference between "token expired, re-login" and "token is forged."
Once cached.conn is set, it's never checked for liveness. If the MongoDB connection drops, all subsequent calls return the dead connection. Consider checking mongoose.connection.readyState before returning the cached connection.
console.log('MongoDB connected successfully') runs on every cold start. In high-traffic serverless environments, this pollutes logs.
Discord rate limits webhooks to ~30 requests per 60 seconds. High-traffic events (votes, site visits) could hit rate limits causing silent failures.
Some embed fields use raw emoji literals (📐, 🌐) while the rest of the file uses the EMOJI constant map.
When user.level >= 100, xpToNext(100) returns Infinity. user.xp / Infinity gives 0, so xpPercent will always be 0 at level 100, even though the user has "completed" leveling. This is misleading.
xp = Math.min(xp, xpToNext(99)) caps overflow XP to the level 99→100 requirement. If a large XP award spans multiple levels, the excess is silently lost.
Was: return this.findOne({ name: new RegExp('^${name}$', 'i') });
The name parameter was interpolated directly into a regex without escaping. Used from api/animals/[id].js with URL params, enabling ReDoS and unintended match behavior (e.g., name = ".*" matches any animal).
Fix applied: Now escapes regex metacharacters before interpolation.
name had both unique: true and index: true. The unique constraint already creates an index.
Fix applied: Removed redundant index: true.
Six individual indexes on attack, defense, agility, stamina, intelligence, and special_attack. For a collection of hundreds of animals, these indexes add write overhead without meaningful query benefit. The compound index { attack: -1, defense: -1 } partially overlaps with the individual attack index.
Same as Animal.js — unique: true already creates an index.
Fix applied: Removed redundant index: true.
If the application sorts by battleRating for leaderboards, there should be an index on this field.
The schema both explicitly defines createdAt with Date.now and enables timestamps: true. Mongoose uses the timestamps option, making the explicit default: Date.now dead code. The explicit definition does serve the purpose of adding the index.
Issue L16 — MEDIUM: pre('find') / pre('findOne') middleware hides deleted messages globally (Lines 80-86)
This automatically filters out deleted messages on every query. Admin/moderation queries cannot retrieve deleted messages without bypassing the model entirely.
Voter IDs stored as embedded arrays mean each message document grows with every vote, risking the 16MB document limit.
voteScore is a denormalized field that must be manually updated via updateVoteScore(). No pre-save hook keeps it in sync if upvotes/downvotes are modified without calling updateVoteScore().
Comments can be empty strings (after trimming) since there's only maxlength but no minlength.
If the application queries rank history by animal name, there's no index to support that query efficiently.
Rank history entries accumulate indefinitely with no TTL index or pruning.
date has index: true on the field AND a separate { unique: true } index. The unique index supersedes the simple one.
Was: Read-modify-write pattern (findOne → modify in memory → save) not atomic. Under concurrent serverless requests, two requests could read the same state and one would overwrite the other's changes, losing visit counts.
Fix applied: Rewrote to use atomic MongoDB operations ($addToSet, $inc, $pull) instead of in-memory modification.
Every unique visitor's IP hash is stored in an array. On a busy site, 7 days × thousands of IPs in a single document can approach the 16MB limit. The includes() check was also O(n).
The parameter is named ipHash but nothing verifies the caller actually hashed the IP. If raw IPs are passed, they're stored in plain text (privacy/GDPR concern).
recordVisit is defined but never invoked anywhere in the codebase.
If a caller forgot .select('+password'), this.password would be undefined and bcrypt.compare would throw a cryptic error.
Fix applied: Added defensive check that throws a clear error message.
User voting history embedded as arrays in the User document. Active users accumulating thousands of votes will bloat the document and impact every query fetching the user.
Same issue as ChatMessage.
Minimum 6 characters with no complexity requirements.
Old username change records are never pruned.
Was: Two countDocuments calls for upvotes and downvotes.
Fix applied: Replaced with a single aggregation query, halving database round trips.
Individual index on animalId is redundant with the compound unique index { animalId, votedBy, voteDate } since animalId is the leftmost prefix.
A user could send extreme timezones (e.g., UTC+14 vs UTC-11) to claim XP for dates not yet reached in UTC.
| File | Issue | Fix |
|---|---|---|
lib/auth.js |
Hardcoded JWT fallback | Throws if missing |
api/auth.js |
Duplicated JWT_SECRET | Now imports from lib/auth.js |
api/auth.js |
Regex injection in username lookups | Escapes metacharacters |
lib/models/Animal.js |
Regex injection in findByName |
Escapes metacharacters |
lib/models/Animal.js |
Redundant index on name |
Removed index: true |
lib/models/BattleStats.js |
Redundant index on animalName |
Removed index: true |
lib/models/SiteStats.js |
Race condition in recordVisit |
Uses atomic MongoDB ops |
lib/models/User.js |
comparePassword no defensive check |
Throws clear error |
lib/models/Vote.js |
Two queries in getVoteCounts |
Single aggregation |
Reviewed all 12 API endpoint files. Found 55+ issues across security, bugs, error handling, input validation, and code quality. The most critical issues involve missing authentication on mutation endpoints, NoSQL/ReDoS injection via unescaped regex, a hardcoded JWT secret fallback, and mass assignment vulnerabilities.
Severity Legend:
- CRITICAL: Exploitable security vulnerability or data-loss bug
- HIGH: Significant bug or security concern
- MEDIUM: Non-trivial issue that should be fixed
- LOW: Code quality or minor concern
The search query parameter is interpolated directly into $regex without escaping regex special characters. An attacker can craft input like .* or (a+)+b to cause ReDoS (Regular Expression Denial of Service) or return unintended data.
query.$or = [
{ name: { $regex: search, $options: 'i' } },
{ scientific_name: { $regex: search, $options: 'i' } },
{ description: { $regex: search, $options: 'i' } }
];
Fix: Escape the search string: search.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') before using in $regex.
Same issue — biome is passed directly to $regex:
query.habitat = { $regex: biome, $options: 'i' };
handlePost creates new animals with zero authentication. Any anonymous user can create arbitrary animals in the database.
{ $sort: { totalStats: -sortOrder } }
sortOrder is -1 for desc and 1 for asc (line 188). Negating it (-sortOrder) reverses the intended order: requesting order=desc actually sorts ascending, and vice versa. Should be { $sort: { totalStats: sortOrder } }.
parseInt(skip) and parseInt(limit) could return NaN for non-numeric input, causing MongoDB errors. No maximum cap on limit (default is 500 but a client can send limit=999999).
diet is extracted from req.query but is never used to filter — dead code.
handleNotification returns 200 even on error, masking failures silently.
Both handlePut and handleDelete modify/destroy data with no authentication or authorization check. Any anonymous user can update or delete any animal.
const updateData = req.body;
delete updateData._id;
Object.assign(animal, updateData);
Only _id is protected. An attacker can overwrite any field including createdAt, updatedAt, or any internal/computed field. Should use an allowlist of permitted fields.
Animal.findByName(id) uses new RegExp('^${name}$', 'i') in the Animal model without escaping. Since id comes directly from the URL path parameter, an attacker can inject regex metacharacters.
const JWT_SECRET = process.env.JWT_SECRET || 'animal-stats-secret-key-change-in-production';
If JWT_SECRET env var is unset, the application uses a well-known secret. Any attacker can forge valid JWT tokens. This is also duplicated in lib/auth.js line 8.
username: { $regex: new RegExp(`^${username}$`, 'i') }
The username query parameter for ?action=user is used in a regex without escaping or validation. Unlike the profile update path (which validates format), this public endpoint accepts arbitrary input.
xpToAward = parseInt(customXp) || 0;
bpToAward = parseInt(customBp) || 0;
Any authenticated user can call POST /api/auth?action=rewards with customXp=500 repeatedly. While individual amounts are capped (line 510), there's no rate limiting or cooldown, allowing unlimited XP farming.
res.status(500).json({ success: false, error: 'Server error. Please try again.' });
Missing return keyword. While this doesn't cause a double-response (it's the last statement in the catch), it's inconsistent with other handlers and could cause issues if code is added after it.
handleMe (line 227), handleGetProfile (line 269), and handleUpdateProfile (line 322) manually decode JWT tokens, while handleRewards (line 469) and handlePrestige (line 588) use the verifyToken helper. This duplicated logic is error-prone — if the token format changes, multiple places need updating.
The signup handler (line 159) doesn't validate email format. Relying solely on the Mongoose schema validator produces Mongoose error messages instead of user-friendly ones.
handleMe omits xpToNext, xpPercentage, prestige, and lifetimeXp fields that are returned by handleGetProfile and handleLogin. This inconsistency can cause client-side bugs.
Anyone can submit arbitrary battle results to manipulate ELO ratings. An attacker can repeatedly submit wins for a chosen animal to inflate its rating.
handleTournamentComplete and handleTournamentQuit accept unauthenticated requests. An attacker can fabricate tournament results, awarding fake placements to animals.
recordMatchupVote has no auth check and no duplicate vote prevention per user. Any client can call this endpoint repeatedly to stuff the ballot for a given matchup.
let matchup = await MatchupVote.findOne({ matchupKey });
// ...
matchup.animal1Votes += 1;
await matchup.save();
Between findOne and save, another request could modify the same document. This read-modify-write pattern is not atomic and can lose votes under concurrent load. Should use $inc with findOneAndUpdate.
Same read-modify-write pattern for winnerStats and loserStats. Two concurrent battle submissions could overwrite each other's rating changes.
Each animal placement calls updateTournamentPlacement and incrementTournamentsPlayed individually (each doing a findOne + save). For a tournament with many participants, this could be 16+ sequential DB operations. Should batch with bulkWrite.
The Mongoose schema is defined inside the API route file rather than in lib/models/. This violates the project's own organizational pattern and makes it harder to reuse or test.
let query = { parentId: null }; // Only get root messages
The query does not exclude messages with isDeleted: true. Soft-deleted messages will still appear in chat. Should add isDeleted: { $ne: true } to the query.
Replies are fetched without checking if the parent or the reply itself is deleted.
Chat message content is stored with only .trim(). No HTML stripping or script tag removal. While XSS mitigation is typically handled client-side (React auto-escapes), if content is ever rendered as raw HTML or used in emails/notifications, stored XSS is possible.
const isOwner = message.authorId.toString() === user.id;
If message.authorId is null/undefined, calling .toString() will throw an uncaught exception within the try-catch, resulting in a 500 error instead of a proper message.
Authenticated users can spam messages with no cooldown or rate limit.
buildMessageTree (line 57) processes all messages including deleted ones since the query doesn't filter them.
if (comment.authorId.toString() !== user.id) {
If comment.authorId is null/undefined, this will throw TypeError: Cannot read properties of null (reading 'toString').
const upvoteIndex = comment.upvotes.findIndex(id => id.toString() === userId);
const downvoteIndex = comment.downvotes.findIndex(id => id.toString() === userId);
The callback parameter id shadows the outer id from req.query (line 307). While functionally correct, this is confusing and could lead to bugs during refactoring.
Unlike chat.js (line 421-422) which checks user.role === 'admin' || user.role === 'moderator', comments can only be deleted by their author. This inconsistency means admins can moderate chat but not comments.
await Comment.deleteMany({ parentId: targetId });
await Comment.deleteOne({ _id: targetId });
Comments are hard-deleted rather than soft-deleted. This is inconsistent with the chat handler which uses soft-delete (isDeleted: true). Hard-deleted data cannot be recovered for moderation review.
Every upvote and downvote triggers a Discord notification (lines 336, 355). On active pages this could flood the Discord channel.
handleVisit increments the site visit counter with no authentication, rate limiting, or deduplication. An attacker can send millions of requests to inflate the visit counter.
const presenceStore = new Map();
If the server restarts or runs multiple instances (e.g., behind a load balancer), presence data is lost or inconsistent. Acknowledged in a comment, but worth flagging.
xpProgress: Math.min(100, Math.round((xpProgress / xpNeeded) * 100)),
If calculateXpForLevel returns 0 (which it does for level 2: 100 + (2-2)*50 = 100, OK, that's fine, but for level 1: 100 + (1-2)*50 = 50), the division is safe. However, if the formula changes or a level produces 0, this would yield Infinity.
function calculateXpForLevel(level) {
return 100 + (level - 2) * 50;
}
This is a local copy of the formula that should come from lib/xpSystem.js. If the formula is updated in one place but not the other, rankings will show incorrect XP progress. Should import from the shared module.
Unlike ping and visit which check req.method, leaderboard and presence respond to any HTTP method (POST, PUT, etc.).
The fight notification and comparison count incrementing has no auth check. An attacker can inflate comparison counts for any animal.
.select('name image attack defense agility stamina intelligence special scientific_name')
The Animal schema defines the field as special_attack (Animal.js line 94), not special. This means animal.special is always undefined, and the total stats calculation on line 141:
(animal.special || 0)
always evaluates to 0, making total stats incorrect for display on the rankings page.
const { animal1, animal2, user } = req.body;
If req.body is undefined, or animal1/animal2 are missing, the code proceeds to call notifyDiscord and database updates with undefined values. No validation before use.
Rank history is saved without await. While this is intentional for performance, if the save consistently fails (e.g., schema mismatch), trends will never work and the error is only logged, not surfaced.
const numAnimals = Math.min(parseInt(count), 10);
If count is a non-numeric string like "abc", parseInt returns NaN, and Math.min(NaN, 10) is NaN. This causes $sample: { size: NaN } which will throw a MongoDB error.
Fix: const numAnimals = Math.min(parseInt(count) || 1, 10);
data: numAnimals === 1 ? animals[0] : animals
If no animals match (empty database or all excluded), animals[0] is undefined, and the response sends { success: true, count: 0, data: undefined }. The data field will be omitted from JSON serialization, which may confuse clients. Should return null explicitly or handle the empty case.
{ name: { $regex: q, $options: 'i' } },
Same issue as animals.js — the search query q is used directly in $regex without escaping. This is exploitable for ReDoS attacks.
limit defaults to 50 but has no upper bound. A client can send limit=1000000 to dump the entire database.
const skip = (parseInt(page) - 1) * parseInt(limit);
If page=0 or page=-5, the skip value becomes negative, which MongoDB will treat as 0. If page is non-numeric, NaN propagation occurs.
const sortField = sort === 'special' ? 'special_attack' : sort;
sortObj[sortField] = order === 'desc' ? -1 : 1;
The sort parameter is used as a MongoDB field name with minimal transformation. While Mongoose provides some protection, this allows sorting by any field including internal ones like _id or __v.
This is a read-only GET endpoint with proper error handling, method validation, and CORS. Minor observation: no caching headers are set, so repeated calls hit the database every time. Adding short-lived caching (e.g., 60s) would improve performance.
if (voteType && !['up', 'down', 'clear'].includes(voteType)) {
If voteType is undefined or null, the entire if-else chain (lines 155-217) falls through without creating, updating, or clearing a vote, but still queries the database and returns a misleading "success" response with action: 'none'.
animalId is not validated as a proper MongoDB ObjectId before being used in queries. If an invalid ID is provided, the database query may throw.
Minor formatting issue.
All 12 files set Access-Control-Allow-Origin: *. While acceptable for a public read-only API, this is problematic for mutation endpoints that use cookies or auth tokens. It means any website can make authenticated requests to your API if the user has a valid token stored client-side.
None of the 12 API files implement server-side rate limiting. Combined with missing auth on many endpoints, this makes the API vulnerable to abuse:
- Spamming animal creation
- Inflating battle ratings / matchup votes / visit counters
- Brute-forcing login credentials
- XP farming via the rewards endpoint
Both api/auth.js (line 25) and lib/auth.js (line 8) have the same hardcoded fallback: 'animal-stats-secret-key-change-in-production'. If the env var is missing in production, tokens are trivially forgeable.
None of the endpoints enforce a maximum request body size. An attacker could send extremely large payloads (e.g., a 100MB JSON body in POST /api/animals) to exhaust server memory.
Some endpoints return { success: false, error: '...' }, others return { success: false, error: '...', message: '...' }, and the global catch in some files includes message: process.env.NODE_ENV === 'development' ? error.message : undefined while others don't. This inconsistency makes client-side error handling unreliable.
AnimalSchema.statics.findByName = function(name) {
return this.findOne({ name: new RegExp(`^${name}$`, 'i') });
};
This shared utility is called from animals.js, animals/[id].js, and potentially other files. The name parameter is never escaped, making all callers vulnerable to regex injection.
- Hardcoded JWT secret (Issues 11, 57) — Ensure
JWT_SECRETenv var is always set in production - Missing auth on mutation endpoints (Issues 3, 8, 18, 19, 41) — Add authentication checks
- NoSQL/ReDoS injection via unescaped regex (Issues 1, 2, 10, 12, 47, 60) — Escape all user input used in regex
- Mass assignment in PUT /api/animals/[id] (Issue 9) — Use field allowlist
- Sort order bug (Issue 4) — Fix negated sort order
- Wrong field name in rankings (Issue 42) — Change
specialtospecial_attack - Deleted messages still shown (Issues 25, 26) — Add
isDeletedfilter - Race conditions (Issues 21, 22) — Use atomic operations
- Add rate limiting (Issue 56) — Implement at middleware level
- NaN propagation (Issue 45) — Add input sanitization