Feature/api client flavor#135
Conversation
…packages to only concern themselves with the opinionated downloading and storage of assets
![github-advanced-security[bot]](https://avatars.gh.lumenfield.work/in/57789?s=60&v=4){kind=small}
| async function hashPassword(email: string, password: string) { | ||
| return crypto | ||
| .createHash("sha256") | ||
| .update(password + email.toLowerCase()) |
Check failure
Code scanning / CodeQL
Use of password hash with insufficient computational effort High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix this problem, replace the insecure use of crypto.createHash('sha256') for password hashing with a secure password hashing function, such as bcrypt. This entails updating the hashPassword function to use bcrypt.hashSync or its async variant. Given that you must not change behavior outside the provided snippet, and only the code in this file, you should implement the secure hash just within the hashPassword function. You will need to import bcrypt at the top. The code must generate a salt (or accept one as input), as recommended for bcrypt. You should ensure consistency with how hashes are generated and used later (e.g., replacing the call on line 46 to use the securely hashed password).
Updates include:
- Add
bcryptimport. - Update the
hashPasswordfunction to use bcrypt (asynchronously, since original use is with await). - (Optionally) Generate a salt within the function if not externally supplied.
| @@ -1,6 +1,7 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import crypto from "node:crypto"; | ||
| import bcrypt from "bcrypt"; | ||
| import { Command } from "@commander-js/extra-typings"; | ||
| import { CarApi, Configuration } from "@iracing-data/api-client-fetch"; | ||
| import { syncCarAssets } from "@iracing-data/sync-car-assets"; | ||
| @@ -11,10 +12,9 @@ | ||
| dotenv.config(); | ||
|
|
||
| async function hashPassword(email: string, password: string) { | ||
| return crypto | ||
| .createHash("sha256") | ||
| .update(password + email.toLowerCase()) | ||
| .digest("base64"); | ||
| const saltRounds = 12; | ||
| // Use bcrypt to securely hash password; include email in hash input if required for protocol. | ||
| return await bcrypt.hash(password + email.toLowerCase(), saltRounds); | ||
| } | ||
|
|
||
| const program = new Command("sync-iracing-car-assets") |
| @@ -18,7 +18,8 @@ | ||
| "@iracing-data/api-client-fetch": "workspace:*", | ||
| "@iracing-data/sync-car-assets": "workspace:*", | ||
| "commander": "^14.0.2", | ||
| "openapi-fetch": "^0.15.0" | ||
| "openapi-fetch": "^0.15.0", | ||
| "bcrypt": "^6.0.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@commander-js/extra-typings": "^14.0.0", |
| Package | Version | Security advisories |
| bcrypt (npm) | 6.0.0 | None |
| export async function hashPassword(email: string, password: string) { | ||
| return crypto | ||
| .createHash("sha256") | ||
| .update(password + email.toLowerCase()) |
Check failure
Code scanning / CodeQL
Use of password hash with insufficient computational effort High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To resolve this issue, we need to replace the use of the general-purpose SHA-256 hashing algorithm for password hashing with a computationally expensive password hashing scheme, such as bcrypt. This change should be done within the hashPassword function in apps/sync-track-assets-cli/src/index.ts (lines 16–21). We will use the bcrypt library, which is a widely accepted method for password hashing in Node.js applications. This will involve:
- Installing and importing the
bcryptpackage (not shown in other parts, so we’ll just add the import line shown). - Modifying the
hashPasswordfunction to usebcrypt.hash()(the async version, since the function is already marked async). - Deciding on a salt factor (e.g., 12 is a good default).
- Removing the manual concatenation with the email, unless required by the downstream API (if needed, can append to the password before passing to bcrypt).
- Ensuring the rest of the code continues to treat the hash as it did previously (as a string).
All changes should be made only to apps/sync-track-assets-cli/src/index.ts, with sufficient context to clearly identify the changed lines (i.e., lines 16-21 must be replaced; also add the import for bcrypt).
| @@ -1,6 +1,7 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import crypto from "node:crypto"; | ||
| import bcrypt from "bcrypt"; | ||
| import { Command } from "@commander-js/extra-typings"; | ||
| import { Configuration, TrackApi } from "@iracing-data/api-client-fetch"; | ||
| import { syncTrackAssets } from "@iracing-data/sync-track-assets"; | ||
| @@ -14,10 +15,10 @@ | ||
| * Compute the Base64‑encoded SHA‑256 hash of (password + email.toLowerCase()). | ||
| */ | ||
| export async function hashPassword(email: string, password: string) { | ||
| return crypto | ||
| .createHash("sha256") | ||
| .update(password + email.toLowerCase()) | ||
| .digest("base64"); | ||
| // It’s typical to use just the password, but if the API requires email+password, keep that | ||
| const saltRounds = 12; | ||
| const toHash = password + email.toLowerCase(); | ||
| return await bcrypt.hash(toHash, saltRounds); | ||
| } | ||
|
|
||
| const program = new Command("sync-iracing-track-assets") |
2 participants
No description provided.