diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 00000000..654bfd34 --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,99 @@ +# For most projects, this workflow file will not need changing; you simply need +# to commit it to your repository. +# +# You may wish to alter this file to override the set of languages analyzed, +# or to provide custom queries or build logic. +# +# ******** NOTE ******** +# We have attempted to detect the languages in your repository. Please check +# the `language` matrix defined below to confirm you have the correct set of +# supported CodeQL languages. +# +name: "CodeQL Advanced" + +on: + push: + branches: [ "main" ] + pull_request: + branches: [ "main" ] + schedule: + - cron: '30 17 * * 4' + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + # Runner size impacts CodeQL analysis time. To learn more, please see: + # - https://gh.io/recommended-hardware-resources-for-running-codeql + # - https://gh.io/supported-runners-and-hardware-resources + # - https://gh.io/using-larger-runners (GitHub.com only) + # Consider using larger runners or machines with greater resources for possible analysis time improvements. + runs-on: ${{ (matrix.language == 'swift' && 'macos-latest') || 'ubuntu-latest' }} + permissions: + # required for all workflows + security-events: write + + # required to fetch internal or private CodeQL packs + packages: read + + # only required for workflows in private repositories + actions: read + contents: read + + strategy: + fail-fast: false + matrix: + include: + - language: javascript-typescript + build-mode: none + # CodeQL supports the following values keywords for 'language': 'actions', 'c-cpp', 'csharp', 'go', 'java-kotlin', 'javascript-typescript', 'python', 'ruby', 'rust', 'swift' + # Use `c-cpp` to analyze code written in C, C++ or both + # Use 'java-kotlin' to analyze code written in Java, Kotlin or both + # Use 'javascript-typescript' to analyze code written in JavaScript, TypeScript or both + # To learn more about changing the languages that are analyzed or customizing the build mode for your analysis, + # see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/customizing-your-advanced-setup-for-code-scanning. + # If you are analyzing a compiled language, you can modify the 'build-mode' for that language to customize how + # your codebase is analyzed, see https://docs.github.com/en/code-security/code-scanning/creating-an-advanced-setup-for-code-scanning/codeql-code-scanning-for-compiled-languages + steps: + - name: Checkout repository + uses: actions/checkout@v4 + + # Add any setup steps before running the `github/codeql-action/init` action. + # This includes steps like installing compilers or runtimes (`actions/setup-node` + # or others). This is typically only required for manual builds. + # - name: Setup runtime (example) + # uses: actions/setup-example@v1 + + # Initializes the CodeQL tools for scanning. + - name: Initialize CodeQL + uses: github/codeql-action/init@v4 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + # If you wish to specify custom queries, you can do so here or in a config file. + # By default, queries listed here will override any specified in a config file. + # Prefix the list here with "+" to use these queries and those in the config file. + + # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs + # queries: security-extended,security-and-quality + + # If the analyze step fails for one of the languages you are analyzing with + # "We were unable to automatically build your code", modify the matrix above + # to set the build mode to "manual" for that language. Then modify this step + # to build your code. + # ℹ️ Command-line programs to run using the OS shell. + # 📚 See https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idstepsrun + - name: Run manual build steps + if: matrix.build-mode == 'manual' + shell: bash + run: | + echo 'If you are using a "manual" build mode for one or more of the' \ + 'languages you are analyzing, replace this with the commands to build' \ + 'your code, for example:' + echo ' make bootstrap' + echo ' make release' + exit 1 + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v4 + with: + category: "/language:${{matrix.language}}" diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..6cefa7f7 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,3 @@ +{ + "codeQL.createQuery.qlPackLocation": "c:\\Users\\enriq\\OneDrive\\Skrivbord\\School\\yh-message-app-fullstack" +} \ No newline at end of file diff --git a/README.md b/README.md index d6aeeb20..5a16a83c 100644 --- a/README.md +++ b/README.md @@ -1 +1,41 @@ -# yh-message-app-fullstack \ No newline at end of file +# yh-message-app-fullstack +Sarah Tjellander +- Har testat formulera texten på inlämning FAS 1 enligt Markdown guide på Dicso + +Nathalie Loyd + +Henrik Söderqvist + +Server.js app.delete rad 238-251 +- Jag har föreslagit en kodändring med förklaring. + +Index.css +- OKad + +Server.js app.get rad 169-180 +- Jag har föreslagt en kodändring med förklaring. + +Sever.js app.patch rad 211-228 +- Lagt till kommentar att meddelanden behöver valideras innan dem sparas i databasen. För undvika att skadlig kod skrivs in. + + +Backend .env.exemple +- dubbelkolla att http adressen endast används i utveckling .ent.exempel och inte i produktionskod. +- Inte hittat något, BASE_API använder https. + + +Server.js app.post rad 76-98 +- Risk: Information Disclosure och Brute Force. Krav: Rate Limiting + +Frontend src i alla .jsx - Lagt kommentar i App.jsx +- Klassiskt utvecklarmisstag. Ta bort: console.log helt och hållet. + +Frontend - server.js: return res.status rad 100-103 +- Föreslagit ändring av felmeddelande för att minksa risken att någon kan se vilka konton som finns. + +Backend - models/Message.js: rad 18 - 20 +- Har lagt in förslag om att lägga in en begrännsning på längden av meddelandet. + +Server.js - app.post login +- SÄKERHETSFÖRBÄTTRING: Generellt felmeddelande för inloggning +- även lagt in ny kod som säkerhetsval \ No newline at end of file diff --git a/backend/.env.example b/backend/.env.example index d417a785..c1891042 100644 --- a/backend/.env.example +++ b/backend/.env.example @@ -2,3 +2,5 @@ PORT=3000 MONGO_URL=connection-string-from-mongodb-atlas JWT_SECRET=your-secret-here FRONTEND_URL=http://localhost:5500 + +# // FRONTEND_URL=http://localhost:5500 Det är en okrypterad adress och inte säker att använda. Är detta endast för utveckling? Säkerställ att det inte används i produktion. \ No newline at end of file diff --git a/backend/models/Message.js b/backend/models/Message.js index dbca0001..cb4e7f24 100644 --- a/backend/models/Message.js +++ b/backend/models/Message.js @@ -15,4 +15,8 @@ createdAt: { }, }) +// Skapa en begränsning på längden av meddelandet. +// Detta är en säkerhetsåtgärd för att förhindra att användare skickar mycket långa meddelanden, +// som i sin tur kan orsaka prestandaproblem eller överbelasta databasen. + export const Message = mongoose.model("Message", messageSchema) diff --git a/backend/server.js b/backend/server.js index c8d0c218..47c6b64c 100644 --- a/backend/server.js +++ b/backend/server.js @@ -80,24 +80,27 @@ app.post("/login", async (req, res) => { $or: [{ username: login }, { email: login }] }) - if (!user) { - return res.status(401).json({ - success: false, - message: "No account found with that username or email", - response: null, - }) - } + // 1. SKAPA EN DUMMY-HASH: Den används bara om användaren INTE hittas. + const dummyHash = "$2b$10$AzR7R.JvG7p0H2A9kYvOLeEa8yI1yZpE8fXfH1g7m7f8i9o0p1q2r" + + // 2. VÄLJ STRÄNG ATT JÄMFÖRA MED: Finns användaren? Ta dess riktiga hash. Finns den inte? Ta dummy-hashen. + const hashToCompare = user ? user.password : dummyHash - const passwordMatch = await bcrypt.compare(password, user.password) - if (!passwordMatch) { + // 3. KÖR BCRYPT: Detta tar alltid ~80-100ms och stoppar timing-attacker + const passwordMatch = await bcrypt.compare(password, hashToCompare) + + // 4. KONTROLLERA OM NÅGOT GICK FEL: + // Om användaren inte fanns ELLER om lösenordet inte matchade, skicka samma fel. + if (!user || !passwordMatch) { return res.status(401).json({ success: false, - message: "Password is incorrect", + message: "Invalid username/email or password", response: null, }) } - const accessToken = jwt.sign( + // 5. LYCKAD INLOGGNING: Hit kommer man om både användaren fanns och lösenordet var rätt! + const accessToken = jwt.sign( { userId: user._id, username: user.username }, process.env.JWT_SECRET, { expiresIn: "2h" } @@ -121,6 +124,57 @@ app.post("/login", async (req, res) => { } }) + + // SÄKERHETSFÖRBÄTTRING: Generellt felmeddelande för inloggning + // Ändra felmeddelande för att inte avslöja om det var användarnamnet eller lösenordet som var felaktigt. + // Detta är en säkerhetsåtgärd för att förhindra att angripare får information om vilka användarnamn som finns i systemet. + // Vi returnerar samma felmeddelande oavsett om det var användarnamnet eller lösenordet som var fel. + // Exempel på ändrat felmeddelande: message: "Invalid login or password" + //Dataläcka med felmeddelande. FRÅN FAS 1: Hot mot pilen "JSON-svar" (I - Information Disclosure): Läckage av känslig data. + // Angriparen får reda på om användaren redan finns eller inte eftersom svaret är "Password is incorrect" eller "No account found with that username or email". + // För att undvika detta bör vi använda ett generellt felmeddelande som inte avslöjar vilken del av inloggningen som misslyckades. + // Givet val för angripare att använda Brute Force/Denail of Service (DoS) i STRIDE, där de kan försöka gissa lösenordet genom att göra många inloggningsförsök. + // Lägga till en Rate Limiter som en spärr specifikt för inloggningen (Krav 4 FRÅN FAS 1). Max 5 försök/15 min per IP-adress. + +//const rateLimit = require("express-rate-limit"); + +// 1. Skapa spärr: +// const loginLimiter = rateLimit({ +// windowMs: 15 * 60 * 1000, +// max: 5, +// message: { success: false, message: "Too many login attempts, please try again later." } +// }); + +// 2. APPLICERA SPÄRR: Lägg till 'loginLimiter' i endpointen +// app.post("/login", loginLimiter, async (req, res) => { +// try { +// const { login, password } = req.body +// const user = await User.findOne({ +// $or: [{ username: login }, { email: login }] +// }) + +// 3. MODIFIERAT FELMEDDELANDE 1: Säg inte att kontot saknas utan generellt meddelande: +// if (!user) { +// return res.status(401).json({ +// success: false, +// message: "Invalid username, email or password", +// response: null, +// }) +// } + +// const passwordMatch = await bcrypt.compare(password, user.password) + +// (!passwordMatch) { +// return res.status(401).json({ +// success: false, +// message: "Invalid username, email or password", +// response: null, +// }) +// } + + + + const isValidId = (id) => mongoose.Types.ObjectId.isValid(id) app.get("/messages", async (req, res) => { @@ -135,6 +189,25 @@ app.get("/messages", async (req, res) => { res.status(500).json({ message: "Could not fetch messages" }) } }) +//Rate Limiting för resursskydd, gäller Denial of Service (överbelastning) i STRIDE. En angripare kan skicka detta anrop 50 000 gånger i sekunden och krascha servern. +//Vi bör lägga till en blockering (en limiter) som stoppar en angripare från att göra för många anrop per minut. +//Vi föreslår att använda Rate Limiting-middleware (express-rate-limit) +//Node.js använder standardverktyget express-rate-limit för att lösa detta. Det läggs till högst upp i filen, och sedan appliceras det på endpoint. + +// 1. Importera verktyget för hastighetsbegränsning (detta görs över app.get) +// const rateLimit = require("express-rate-limit"); + +// 2. Definiera reglerna: Max 100 anrop per 15 minuter från samma IP (detta läggs under Const rateLimit) +// const messageLimiter = rateLimit({ +// windowMs: 15 * 60 * 1000, // 15 minuter i millisekunder +// max: 100, // Begränsa varje IP till 100 anrop per fönster +// message: { error: "Too many requests, please try again later." } +// }); + +// 3. Lägg till 'messageLimiter' som ett filter i din existerande app.get-kod +// app.get("/messages", messageLimiter, async (req, res) => { + + app.post("/messages", authenticateUser, async (req, res) => { const message = new Message({ message: req.body.message, user: req.user._id }) @@ -165,11 +238,21 @@ app.patch("/messages/:id", authenticateUser, async (req, res) => { } }) -app.delete("/messages/:id", async (req, res) => { +// För att säkerställa att endast ägaren av ett meddelande kan redigera det, kontrollerade vi att i PATCH-routen för uppdatering av meddelanden. +// Detta fanns redan och den jämför den inloggade användarens ID (från JWT-token) med det userId som är kopplat till meddelandet i databasen. + +// Dock saknas Krav 2 (Indatavalidering) FRÅN FAS 1 i både app.post och app.patch: +// Meddelanden valideras inte i båda endpoints. Via meddelanden kan en angripare skicka skadlig kod som sparas blint rakt in i databasen. +// Applikationen är helt öppen för XSS. Modifiera koden så meddelandet städas innan den skickas till databasen. + + +app.delete("/messages/:id", async (req, res) => { if (!isValidId(req.params.id)) return res.status(400).json({ error: "Invalid message ID" }) try { const message = await Message.findById(req.params.id) if (!message) return res.status(404).json({ error: "Message not found" }) + + await message.deleteOne() res.status(204).send() } catch (error) { @@ -177,6 +260,18 @@ app.delete("/messages/:id", async (req, res) => { } }) +// Hit kommer koden BARA om kontrollen nedan var godkänd (flyttad text) +//Strikt behörighetskontroll vid dataändring. Motverkar E (Elevation of Privilege) i STRIDE inom Express. +// Koden raderar meddelandet utan att kontrollera vem användaren är. +// Vi måste modifiera koden så att den jämför den inloggade användarens ID med meddelandets userId innan raderingen tillåts. +// 1. Vi lägger till "authenticateUser" här för att tvinga fram inloggning och få fram användarens ID +// 2. NY KONTROLL: Vi jämför meddelandets ägare med den inloggade användaren. + // Vi gör om ID till text (.toString()) för att datorn ska kunna jämföra dem korrekt. + // if (message.user.toString() !== req.user.userId.toString()) { + // // Om det INTE är samma person, stoppar vi anropet med felkod 403 (Förbjudet) + // return res.status(403).json({ error: "You are not authorized to delete this message" }) + + app.listen(PORT, () => { console.log(`Listening on port ${PORT}`) }) diff --git a/codeql-custom-queries-javascript/codeql-pack.lock.yml b/codeql-custom-queries-javascript/codeql-pack.lock.yml new file mode 100644 index 00000000..fd2051e9 --- /dev/null +++ b/codeql-custom-queries-javascript/codeql-pack.lock.yml @@ -0,0 +1,30 @@ +--- +lockVersion: 1.0.0 +dependencies: + codeql/concepts: + version: 0.0.25 + codeql/controlflow: + version: 2.0.35 + codeql/dataflow: + version: 2.1.7 + codeql/javascript-all: + version: 2.7.2 + codeql/mad: + version: 1.0.51 + codeql/regex: + version: 1.0.51 + codeql/ssa: + version: 2.0.27 + codeql/threat-models: + version: 1.0.51 + codeql/tutorial: + version: 1.0.51 + codeql/typetracking: + version: 2.0.35 + codeql/util: + version: 2.0.38 + codeql/xml: + version: 1.0.51 + codeql/yaml: + version: 1.0.51 +compiled: false diff --git a/codeql-custom-queries-javascript/codeql-pack.yml b/codeql-custom-queries-javascript/codeql-pack.yml new file mode 100644 index 00000000..1c5c8b31 --- /dev/null +++ b/codeql-custom-queries-javascript/codeql-pack.yml @@ -0,0 +1,7 @@ +--- +library: false +warnOnImplicitThis: false +name: getting-started/codeql-extra-queries-javascript +version: 1.0.0 +dependencies: + codeql/javascript-all: ^2.7.2 diff --git a/codeql-custom-queries-javascript/example.ql b/codeql-custom-queries-javascript/example.ql new file mode 100644 index 00000000..c9770d9c --- /dev/null +++ b/codeql-custom-queries-javascript/example.ql @@ -0,0 +1,12 @@ +/** + * This is an automatically generated file + * @name Hello world + * @kind problem + * @problem.severity warning + * @id javascript/example/hello-world + */ + +import javascript + +from File f +select f, "Hello, world!" \ No newline at end of file diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index c668778f..ca8e00b3 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -65,14 +65,24 @@ export const App = () => { mode={modal} onClose={() => setModal(null)} onSuccess={(data) => { - console.log("User logged in:", data) + // console.log("User logged in:", data) setUser(data) setModal(null) }} /> )} + +//Kod körs direkt i användarens webbläsaren. Koden skriver ut hela objektet, inkl hemligt accessToken. Vem som helst kan öppna webbläsarens utvecklarverktyg och se det. +//Klassiskt misstag att logga känslig information i frontend. Ta bort console.log som skriver ut hela användarobjektet inklusive JWT-token. + + {error &&

{error}

} - + { } } +//Kod körs direkt i användarens webbläsaren. Koden skriver ut hela objektet, inkl hemligt accessToken. Vem som helst kan öppna webbläsarens utvecklarverktyg och se det. +//Klassiskt misstag att logga känslig information i frontend. Ta bort console.log, två rader. + + return (