Skip to content

someli-api — code inspection

Audit date: 2026-05-17. Lens: concrete code-level issues not already covered by someli-doc/audit/someli-api/security.md and friends.

Summary

19 findings: 4 Critical, 4 High, 7 Medium, 4 Low. Three recurring themes dominate: (1) the main routes/routes.js mounts hundreds of endpoints on the raw Express app without any auth middleware, making most of them unauthenticated; (2) string-interpolated SQL throughout — several instances are directly exploitable, not merely theoretical; (3) m.password (bcrypt hash) is selected and forwarded in multiple list/search responses. Individual critical bugs include a plaintext password comparison that bypasses bcrypt on the login fallback path, a ReferenceError on every call to /RemoveSubject, and a permanent job-lock in the Facebook publish cron.

Critical

C1. Routes registered in apiRoutes.prototype.init have no auth middleware

  • File: routes/routes.js:237-242 (init), server.js:171-174 (mount point)
  • What: apiRoutes.prototype.init assigns router = self.app (the raw Express app, not a sub-router). There is no router.use(auth) call anywhere inside init. The only route with a guard is GET / which uses methods.ensureToken. The 700+ other endpoints registered in this file — including /getAllMemberslist, /getAllConversationlist, /searchAccountUser, /searchAllMembers, /getUserdetail:userid, /getMemberschatMessages, /bni-corporate-profile, /confirmationDetails, and all social-link and AI endpoints — are accessible to unauthenticated callers. By contrast, routes/auth.js and routes/paddle.js both open with router.use(auth).
  • Why it matters: Any unauthenticated caller can enumerate all members (GET /getAllMemberslist returns the entire tMember table), read all conversations (GET /getAllConversationlist), search users (POST /searchAllMembers), and invoke AI-generation endpoints that drive up Bedrock/Gemini cost. The m.password exposure findings below (C4) are exploitable through these same unauthenticated list endpoints.
  • Fix: Inside apiRoutes.prototype.init, add router.use(require('../middlewares/auth')) immediately after var router = self.app — or better, convert to a proper Express Router. Also add the auth middleware to the server-level mount for the routes that intentionally must be public (login, register, webhooks, FB data deletion).

C2. Plaintext password fallback in /webauthenticate and /authenticate bypasses bcrypt

  • File: routes/routes.js:6603-6611 (webauthenticate), routes/routes.js:6665-6673 (authenticate personnel branch)
  • What: After bcrypt.compare(reqObj.password, result[0].password, fn), if checkUser is falsy, the code falls through to else if (reqObj.password == result[0].password) (line 6603 in /webauthenticate, line 6665 in /authenticate personnel branch). This compares the submitted plaintext password against the stored bcrypt hash using string equality — which always fails — but more dangerously it creates a code path that would succeed if a user were created with a stored plaintext password (possible via older code paths or direct DB writes). It also means any bcrypt error causes a silent fallthrough to this branch.
  • Why it matters: The intent is clear (commented-out code around lines 1596-1614 shows this was the old logic). The live code should never reach this branch with a correct hash, but it exists as a landmine: a hash/store bug elsewhere could silently grant login on plaintext match. The personnel login path at line 6665 is actively exercised.
  • Fix: Remove the else if (reqObj.password == result[0].password) branch entirely from both endpoints.

C3. SQL injection in Stripe and Paddle webhook log inserts

  • File: routes/routes.js:17804, 17806, 20330, 20332, 20370, 20372
  • What: All four webhook handlers (/stripe_webhooks, /paddle_sandbox_webhooks, /paddle_production_webhooks, /paddle_Admin_webhooks) log events and errors by string-interpolating JSON.stringify(event), event.type, event.eventType, and err.message directly into con.query(INSERT INTO ...) calls. Example at line 17804: 
    con.query(`INSERT INTO tStripe_webhooks(...) VALUES ('${event.type}', '${JSON.stringify(event)}', ...)`);
    Stripe event payloads are attacker-controlled (anyone can POST to the webhook URL; signature verification prevents legitimate bypass, but the INSERT on the failure path at line 17806 runs with event = undefined, and err.message comes from the Stripe SDK which may embed attacker-supplied content from the raw request). Paddle failure path at line 20332 similarly interpolates event (undefined at that point) and err.message.
  • Why it matters: An attacker who can cause webhook signature failure can inject SQL via the error message captured in the failure-path INSERT. The Stripe failure INSERT at line 17806 calls JSON.stringify(event) where event is undefined, producing the string "undefined" — that INSERT silently writes garbage but also demonstrates the path executes pre-signature-check.
  • Fix: Use parameterized queries for all webhook log inserts. Replace the INSERT calls with con.query('INSERT INTO tStripe_webhooks(...) VALUES (?, ?, ?, ?, ?)', [event?.type, JSON.stringify(event), ..., 'Inserted']).

C4. m.password (bcrypt hash) returned in API list and search responses

  • File: routes/routes.js:601 (/searchAccountUser downline sub-query), routes/auth.js:16525, 16556, 16715, 16767 (member list endpoints), routes/routes.js:4172 (/getUserdetail)
  • What: Multiple endpoints SELECT m.password from tMember and include it in the JSON response sent to the client. Examples:
  • routes/routes.js:601: downline query inside /searchAccountUser explicitly selects m.password.
  • routes/auth.js:16525, 16556, 16767: member-list queries for admin views select m.password alongside '12345' AS auth_pass.
  • routes/routes.js:4172: /getUserdetail:userid selects m.password directly.
  • Why it matters: Exposing bcrypt hashes enables offline cracking. Combined with C1 (unauthenticated routes), /searchAccountUser and /getAllMemberslist return passwords to any caller. Even without C1, admin endpoints should never ship password hashes to the frontend.
  • Fix: Remove m.password from every SELECT that feeds an API response. If password-check is needed server-side, fetch it in a separate query not included in the response object.

High

H1. ReferenceError: userId used before declaration in /RemoveSubject

  • File: routes/routes.js:365
  • What: The handler calls checkuseraccount(userId, selaccountId) at line 365 but userId is never declared in that handler's scope. In the auth.js sub-router it would come from res.userId, but in routes.js (which uses self.app without an auth middleware) there is no destructure of res.userId before this call. JavaScript will throw ReferenceError: userId is not defined on every request to POST /RemoveSubject.
  • Why it matters: This route is completely broken — every call throws an unhandled exception that results in a 500 with no response (because the error propagates to the catch block at the try level which does not have an explicit fallback res.send). Users cannot remove subjects.
  • Fix: Add const { userId } = res; at the top of the handler (consistent with other routes in the same file that use this pattern, e.g., line 1505, 1994).

H2. Hardcoded LinkedIn OAuth client secret

  • File: routes/routes.js:2211-2212 and routes/routes.js:9052-9053
  • What: The LinkedIn OAuth client secret "Cu1nqKFcGY2PMget" is hardcoded in source at two call sites (/linkedinauth and a second OAuth exchange handler). The client ID "86j3m5i3znecqk" is also hardcoded. These are not listed in security.md's hardcoded-secrets table (which covers Slack, Polotno key, and session secret).
  • Why it matters: Any developer with read access to the repo can impersonate the Someli LinkedIn app, exchange codes, and obtain LinkedIn access tokens for any user who has authorized the app. Rotation requires a code change and redeployment.
  • Fix: Move both values to .env (LINKEDIN_CLIENT_ID, LINKEDIN_CLIENT_SECRET) and read via conf.js.

H3. SQL injection via memberId/conversationId in handleChatMessage

  • File: routes/routes.js:123
  • What: The SQL insert for chat messages parameterizes only message (?) but interpolates memberId and conversationId directly: 
    const sql = `INSERT INTO tMessages (...) VALUES (?, ${memberId}, ${conversationId}, NOW(), NOW())`;
    Both memberId and conversationId come from data, which originates from req.body (see line 2168-2169). No type-casting or validation is performed before interpolation.
  • Why it matters: Sending memberId = "1); DROP TABLE tMessages; --" would execute arbitrary SQL. The /getMemberschatMessages endpoint (line 2166) is in routes.js and therefore currently unauthenticated (C1).
  • Fix: Use ? placeholders for all three values.

H4. isOnProcess lock permanently stuck on uncaught error in job_facebook_publish.js

  • File: job_facebook_publish.js:36-47 (cron), job_facebook_publish.js:91 (publish call)
  • What: The cron sets isOnProcess = true at line 39 and then calls post_cp_feeds() at line 41 without .catch(). Inside post_cp_feeds, publish(sql) is called at line 90 as a fire-and-forget (no await, no .catch()). If publish throws before reaching any of the isOnProcess = false assignments, the flag stays true permanently. The cron then skips every subsequent tick, silently halting all Facebook publishing for the lifetime of the PM2 process.
  • Why it matters: Any unexpected DB or network error during the publish flow (not caught in the try block inside request(...) at line 236) freezes the job. There is no watchdog, no timeout reset, no alerting.
  • Fix: Wrap the entire body of post_cp_feeds() in a try/finally that guarantees isOnProcess = false. Similarly add .catch(err => { isOnProcess = false; console.error(err); }) to the post_cp_feeds() call site.

Medium

M1. SQL injection in /searchAllMembers via req.body.username

  • File: routes/routes.js:648
  • What: SELECT * FROM tMember WHERE username like '%${req.body.username}%' — req.body.username is interpolated with no escaping or parameterization.
  • Why it matters: A request with username = "' OR '1'='1 returns all members. Combined with C1, this endpoint is unauthenticated.
  • Fix: Use a parameterized LIKE ? with the wildcard appended in application code: con.query('SELECT ... WHERE username LIKE ?', [%${req.body.username}%]).

M2. SQL injection in /searchAccountUser via search parameter

  • File: routes/routes.js:534
  • What: queryConditions += \ AND (a.accountName LIKE '${search}%')`—searchcomes directly fromreq.body.search` with no sanitization.
  • Fix: Parameterize: AND a.accountName LIKE ? with [search + '%'] in the values array.

M3. PII and credentials logged to stdout in production login paths

  • File: routes/routes.js:6585, 6647, 1576
  • What:
  • Line 6585: console.log("ress", result, checkUser, result?.[0].password, reqObj.password) — logs the bcrypt hash AND the submitted plaintext password.
  • Line 6647: console.log("resp", employeeQuery, checkUser, employeeQuery?.[0].password, reqObj.password) — same for personnel login.
  • Line 1576: console.log("ress", result, result?.[0].password, reqObj.mfa_code, ...) — logs password hash and MFA code. These all ship to pm2 logs in production.
  • Why it matters: Anyone with server log access (ops team, logging aggregator, anyone who runs pm2 logs) reads plaintext submitted passwords. This is a credential-exposure path independent of the DB.
  • Fix: Remove these console.log calls, or at minimum remove password and mfa_code from the logged objects.

M4. tempLogin branch does not return after sending 403

  • File: routes/routes.js:6695-6709
  • What: The tempLogin branch checks if (!verified) and calls res.send(responseObj) but does NOT return. Execution continues to line 6702 where verified.userId is accessed — this throws TypeError: Cannot read properties of null on every invalid token, which Express catches and sends a 500.
  • Why it matters: A caller with a deliberately malformed token sees a 500 instead of the intended 403, leaking that a code path errored. More importantly, the SELECT sid FROM tMember WHERE isDeleted='' AND id='${verified.userId}' at line 6702 contains a type-coercion bug (isDeleted='' instead of isDeleted=0) and verified.userId is attacker-controlled on the injection path — though the immediate TypeError prevents exploitation here.
  • Fix: Add return after res.send(responseObj) on line 6700.

M5. /desigrauthenticate performs plaintext password comparison in SQL

  • File: routes/routes.js:14225
  • What: conditions = \${conditions} AND m.password='${reqObj.password}'`— the password is both compared in plaintext (not hashed) and injected directly into the SQL string. This endpoint is inroutes.js` and is unauthenticated (C1).
  • Why it matters: Any value of reqObj.password containing SQL metacharacters (e.g., ' OR '1'='1) authenticates as any user. Passwords stored in this table must be plaintext for this to work, indicating a separate legacy auth model with no hashing.
  • Fix: If this endpoint is still in use, hash the submitted password before comparison and use a parameterized query. If it's legacy/unused, delete it.

M6. sociallinkOld: tMemberAuth detail column updated with interpolated JSON containing user-supplied OAuth tokens

  • File: routes/routes.js:479
  • What: actions.queryAll({ q: \UPDATE tMemberAuth SET detail='${JSON.stringify(socialdata)}', ...` })—socialdatacontains user-suppliedaccess_token,name,email, and other fields fromreq.body`. Any single-quote in those fields breaks the query; a crafted value enables SQL injection into the UPDATE.
  • Fix: Use a parameterized UPDATE with ? placeholders.

M7. paddle.js /subscriptionList: Paddle customer_id values interpolated into SQL IN() clause

  • File: routes/paddle.js:45-50
  • What: customer_id values fetched from the Paddle API are string-interpolated into an SQL IN() clause: 
    const customerIds = filteredData.map((x) => `'${x.customer_id}'`).join(',');
connection.execute(`... WHERE ... b.customer_id IN (${customerIds})`);
    Paddle customer IDs are typically alphanumeric, but any change in Paddle's ID format or a compromised upstream response could inject SQL. The data originates from a third-party API with no validation layer.
  • Fix: Use ? placeholders with an array: WHERE b.customer_id IN (${customerIds.map(() => '?').join(',')}) and pass the array as values.

Low / nits

L1. parseSignedRequest declared twice — inner version with stronger validation shadowed

  • File: routes/routes.js:261 (inside init, proper HMAC-SHA256 algorithm check + timingSafeEqual) and routes/routes.js:343 (file scope, uses .equals() instead of timingSafeEqual, no algorithm check)
  • What: fb_data_deletion at line 318 calls parseSignedRequest — at that point in the closure, the inner function (line 261) is what's in scope. But the outer declaration at line 343 is dead code for any callers in the module scope. The inner version is the safer one; the outer is the weaker one. This is confusing but not immediately exploitable because the inner version takes precedence within init.
  • Fix: Delete the outer parseSignedRequest at line 343.

L2. downloadimages uses forEach(async ...) — async callbacks not awaited

  • File: routes/routes.js:145-186
  • What: codeChild.forEach(async (c, index) => { ... }) — forEach does not await async callbacks. The function returns defaultCodejson before any of the image-resolving await calls (e.g., the 'Image Library' case at line 167 uses await con.query(...)) complete. The returned JSON will contain unresolved/original image references rather than the resolved S3 URLs.
  • Fix: Replace forEach with for...of loop or await Promise.all(codeChild.map(async ...)).

L3. Unbounded SELECT * on high-cardinality tables

  • File: routes/routes.js:487 (SELECT * FROM tMember WHERE isDeleted = 0), routes/routes.js:682 (SELECT * FROM tConversation WHERE isDeleted = 0)
  • What: Both queries have no LIMIT clause. As the user base grows these return the entire table into memory on every request.
  • Fix: Add pagination (LIMIT/OFFSET or cursor) and select only required columns.

L4. chatSessions Map grows without bound

  • File: routes/routes.js:133
  • What: const chatSessions = new Map() is populated (presumably) but never pruned. There are no chatSessions.delete(...) calls visible in the file, and no TTL. Over time this leaks memory proportional to the number of chat interactions since last restart.
  • Fix: Either use it properly with cleanup logic, or remove it if it's dead code.

Cross-cutting observations

  1. Two separate MySQL connection models coexist in every route file. Each of routes.js, auth.js, and paddle.js creates its own sync-mysql connection (con) and its own mysql2/promise pool (connectionPromise). These are in addition to the db connection managed by modules/dbDriver. The codebase has at minimum 3 separate connection objects per request-handling module, none of which share pool limits. Under load this can exhaust the MySQL max_connections limit silently.

  2. String-interpolated SQL is the norm, not the exception. Dozens of queries throughout routes.js and auth.js use template literals with user or DB-derived values. The parameterized-query pattern (using ?) is used in some newer code (e.g., bni-corporate-profile, confirmationDetails) but has not been applied retroactively. Any future refactor should treat every con.query(\...\${req.body...}`...)` as a potential injection site.

  3. async/await + callback mixing creates invisible race conditions. bcrypt.compare (a callback-based API) is called with await at lines 6408, 6584, 6646 — but await-ing a function that takes a callback returns undefined immediately; the callback fires asynchronously after the route handler has already returned. The code only works because res.send is called inside the callback before Express times out the response. If the callback ever fires after another res.send earlier in the same request (e.g., if a preceding guard calls res.send and the callback fires late), Node.js will throw Cannot set headers after they are sent. This pattern is latent in all three login endpoints.

  4. Production server silently starts without routes on DB failure. server.js:189-196 deliberately starts the server with no routes if DB connection fails in production (NODE_ENV === 'production'). Health checks return 200, but all API calls return 404. This makes a DB outage at startup invisible to load-balancer health checks — traffic is routed to a broken instance.