Potential path traversal bug due to imcomplete path sanitization
Nobody has claimed this yet.
Assessment
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Newbie friendliness
- 35/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Stale
- Tech stack
- express, javascript, node.js
Research direction
Start by locating the path sanitization logic containing filename.indexOf(root) !== 0 and reproduce the two curl --path-as-is requests from the issue against the homepage example. Check that valid files remain accessible while paths escaping the configured directory, including sibling directories sharing its prefix, are denied; add or update coverage if the project has relevant tests.
Written by the indexing model from the issue text.
Description
Hi,
We would like to report a potential path traversal bug: When express-static handle user input, it fails to properly sanitize user input, so that it is possible attackers can still access path out of desired scope in some conditions.
e.g., we can demonstrate the possible attack using the code provided in the project homepage
const express = require('express');
const serve = require('express-static');
const app = express();
app.use(serve(__dirname + '/public'));
const server = app.listen(3000, function(){
console.log('server is running at %s', server.address().port);
});
In the above code, if you send
curl --path-as-is "http://localhost:3000/../1.txt This illegal request will be denied as expected
however, if you send
curl --path-as-is "http://localhost:3000/../public_other/1.txt This illegal request will be processed . This is because the path sanitization logic in express-static only checks if the path starts with a prefix. In this way, if there are any other dictionaries under the same prefix, they are also unexpectedly exposed. This bug is in fact similar to this known vulnerability (https://gist.github.com/lirantal/c80b28e7bee148dc287339cb483e42bc).
if(filename.indexOf(root) !== 0) return next();
Please consider a fix. Thanks!
- Dominant language
- JavaScript
- Stars
- 2
- Forks
- 5
- PR merge metrics
- No merged PRs in 30d
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Similar issues
-
Notifications pagination accepts NaN and negative LIMIT/OFFSETPossibly taken @Senthil455 claimed this today. OpenBUGS MEDIUM
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
elixpo/blogs.elixpo#419 · 2 comments ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 72/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
immich-app/immich#32426 ·
Maintainers usually reply within 1 day
-
automated issue report
Difficulty 1/5 Under an hour Newbie friendliness 70/100
lirantal/discoprint#45 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
scanaislop/aislop#482 ·
Maintainers usually reply within 1 day