Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding login endpoint #2

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Adding login endpoint #2

wants to merge 5 commits into from

Conversation

unionfindbee
Copy link
Owner

This PR introduces a SQL Injection new login endpoint which Mayhem for API detects

Copy link

github-actions bot commented Nov 1, 2023

Mayhem for API Automated API Testing Report

❗ 1 Errors Found

Rule Method Path Details
Internal Server Error GET /login ↗️

✔️ 🎆 0 Warnings Found


Testing details and issue reproduction found at https://app.mayhem.security/forallsecure-demo/mapi-node-example/node/27

Copy link

codecov bot commented Nov 1, 2023

Codecov Report

Attention: 5 lines in your changes are missing coverage. Please review.

Comparison is base (1ea6cdb) 88.23% compared to head (42e85cc) 81.57%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main       #2      +/-   ##
==========================================
- Coverage   88.23%   81.57%   -6.66%     
==========================================
  Files           1        1              
  Lines          17       38      +21     
==========================================
+ Hits           15       31      +16     
- Misses          2        7       +5     
Flag Coverage Δ
unit-tests 50.00% <19.04%> (-38.24%) ⬇️
vulnerability-tests 81.57% <76.19%> (-6.66%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
app.js 81.57% <76.19%> (-6.66%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


db.get(query, [], (err, row) => {
if (err) {
return res.status(500).send(`{"error": "${err.stack}"}`);

Check warning

Code scanning / CodeQL

Exception text reinterpreted as HTML Medium

Exception text
is reinterpreted as HTML without escaping meta-characters.
Exception text
is reinterpreted as HTML without escaping meta-characters.
Comment on lines +27 to +41
app.get('/login', (req, res) => {
const { email, password } = req.query;

if (!email || !password) {
return res.status(400).send('Email and password are required');
}
const query = `SELECT * FROM users WHERE email = '${email}' and password = '${password}'`;

db.get(query, [], (err, row) => {
if (err) {
return res.status(500).send(`{"error": "${err.stack}"}`);
}
return res.send('Login successful');
});
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a database access
, but is not rate-limited.

// Login endpoint (Unsafe)
app.get('/login', (req, res) => {
const { email, password } = req.query;

Check warning

Code scanning / CodeQL

Sensitive data read from GET request Medium

Route handler
for GET requests uses query parameter as sensitive data.
}
const query = `SELECT * FROM users WHERE email = '${email}' and password = '${password}'`;

db.get(query, [], (err, row) => {

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources High

This query string depends on a
user-provided value
.
This query string depends on a
user-provided value
.
const attachmentPath = path.join(__dirname, 'attachments', attachmentName);

// Check if file exists
if (!fs.existsSync(attachmentPath)) {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
}

// Read the file and send it in the response
fs.readFile(attachmentPath, (err, data) => {

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.
Comment on lines +47 to +65
app.get('/attachment/:name', (req, res) => {
// This line directly takes the user input and appends it to the directory path
const attachmentName = req.params.name;
const attachmentPath = path.join(__dirname, 'attachments', attachmentName);

// Check if file exists
if (!fs.existsSync(attachmentPath)) {
return res.status(404).send('Attachment not found');
}

// Read the file and send it in the response
fs.readFile(attachmentPath, (err, data) => {
if (err) {
return res.status(500).send('Error reading file');
}
res.setHeader('Content-Type', 'text/plain');
res.send(data);
});
});

Check failure

Code scanning / CodeQL

Missing rate limiting High

This route handler performs
a file system access
, but is not rate-limited.
This route handler performs
a file system access
, but is not rate-limited.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant