K1ng04 / Web-App

CRUD operations using Javascript, Express and NodeJS
1 stars 1 forks source link

App Dev Code Review #3

Open tleblanc159 opened 3 years ago

tleblanc159 commented 3 years ago

index.html - line 30: Importing the app.js file here is unnecessary. The app.js file contains server-side (NodeJS) code. This code is indirectly called by the client code (web page) through API calls (HTML form POST, PUT, DELETE) and therefore direct access to this code by the client is not required and possibly insecure.

(recommendation) index.html - lines 23, 26 - It is better to use CSS to achieve line breaks due to the fact that HTML is meant to represent the content of the web page while CSS is meant to represent the presentation of said content. A line break contains no content and is purely for presentation. Creating this spacing is trivial with CSS in this case, but it may be worthwhile to learn a technology such as CSS Grid for more complex use cases.

app.js - SQL queries - The SQL queries you construct using interpolated, unsanitized user input seems to be vulnerable to SQL injection attacks. This means that a malicious user could harm your database, data, and possibly even compromise security: https://www.esecurityplanet.com/threats/how-to-prevent-sql-injection-attacks/

General - Typically, you would want to separate your API code (NodeJS) from your client code (web page). This decoupling adds many benefits to the software itself as well as the SDLC as a whole such as: independent updates/deployments for either codebase, more sensical code architecture, ability to use the API with any client codebase, etc.

K1ng04 commented 3 years ago

app.js - SQL queries - The SQL queries you construct using interpolated, unsanitized user input seems to be vulnerable to SQL injection attacks. This means that a malicious user could harm your database, data, and possibly even compromise security: https://www.esecurityplanet.com/threats/how-to-prevent-sql-injection-attacks/

I think the workaround in terms added security and protection against sql injection would be to use Prepared statements.

Thank you!