-
-
Notifications
You must be signed in to change notification settings - Fork 420
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
Add Local File Inclusion Vulnerability #293
base: master
Are you sure you want to change the base?
Conversation
@@ -86,8 +86,8 @@ <h2>How can Vulnerability Scanning Tools use VulnerableApp?</h2> | |||
<br/> | |||
<b>Following are the endpoints exposed: </b> | |||
<ol> | |||
<li><a href="/scanner">Scanner Endpoint</a></li> | |||
<li><a href="/sitemap.xml">SiteMap Endpoint</a></li> | |||
<li><a href="/VulnerableApp/scanner">Scanner Endpoint</a></li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot this fix.
@@ -22,7 +22,7 @@ | |||
<!-- TODO Add active to mode when clicked--> | |||
<div class="navbar navbar-item"> | |||
<div class="navbar-item-menu active" id="home"> | |||
<a href="/">Home</a> | |||
<a href="/VulnerableApp">Home</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for the fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Ivan12273,
Thanks for the PR, however, I have few suggestions:
- we should create a UI where users can give input and that input causes LFI. Something similar to what we have for Command Injection or other vulnerability levels.
- For creating a UI, we need a particular theme such that users related to real-world LFI vulnerabilities. For this, have a look at other vulnerable applications such as DVWA.
- Please add more Vulnerability levels associated with Blacklist, wrongly implemented whitelisting, different protocol bypasses, and 1 or 2 secure implementations such that we can better judge a scanner.
thanks,
Karan
@@ -31,7 +31,7 @@ | |||
Mode | |||
</div> | |||
<div class="navbar-item-menu"> | |||
<a href="https://github.com/SasanLabs/VulnerableApp" target="_blank"><img src="/images/GitHub-Mark-32px.png" /></a> | |||
<a href="https://github.com/SasanLabs/VulnerableApp" target="_blank"><img src="images/GitHub-Mark-32px.png" /></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing
Hello @preetkaran20 then it would be fine if I add an input where the user write the url? |
Didn't get your question fully, however, I would suggest not take URL as input because that doesn't seem like a real-world use case, check some applications like DVWA, etc to see how are they taking the input. We are fine with showing the output below the input. |
Hello @preetkaran20 I did something similar as DVWA, this is how it works: I added a button for verify the URL: For the first level if the user writes a path like this http://localhost:9090/VulnerableApp/?file=secretFiles/passwords.txt this is going to happen: In other cases the app will show: I was only able to add another blacklist vulnerability and a secure implementations, sorry, I hope that's okay. Best regards! |
@Ivan12273, |
Alright, I will change it for an input box. |
@preetkaran20 Hi again, I changed it for the input box :) |
I don't think input as a URL to an application for LFI is right. It might be the case for RFI or SSRF. For LFI we just need file name only. As we see in DVWA, a PHP file is included based on an input parameter. My suggestion is to think of a use case first. Please go through some use cases from multiple places like https://www.neuralegion.com/blog/local-file-inclusion-lfi/ or https://www.netsparker.com/blog/web-security/local-file-inclusion-vulnerability/. An example use-case at the top of my mind is: Code:
Now in this, user can change filename as "/etc/passwd" and that can cause LFI. You are free to ignore my suggested example usecase. Earlier Input box suggestions was little wrong from my side as there is nothing much we can do with input box. |
@@ -0,0 +1,6 @@ | |||
<h2>TOP SECRET PASSWORDS</h2> | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we don't need a special file, use /etc/passwd and equivalent for windows.
sanitized, allowing directory traversal characters (such as dot-dot-slash) | ||
to be injected. | ||
<br /> <br /> | ||
Try to find the password file inside the secretFiles folder using the URL param: <i>file</i> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line sounds like a challenge, which doesn't align with our vulnerable applications mission. Consider this application as a general application which has flaws and let scanners explore and find them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will change it.
data = data + reader.nextLine(); | ||
} | ||
payload.append(data); | ||
reader.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please close the reader in finally block or better use try with resourcr
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright!
value = LevelConstants.LEVEL_2, | ||
descriptionLabel = "LFI_URL_PARAM_BASED_INJECTION_WITH_VALIDATION_ON_FILE", | ||
htmlTemplate = "LEVEL_1/LFI") | ||
public ResponseEntity<String> getVulnerablePayloadLevelUnsecureLevel2( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please try queryparam by name injection instead of map.
new File( | ||
"src/main/java/org/sasanlabs/service/vulnerability/lfi/" | ||
+ queryParameterURL); | ||
Scanner reader = new Scanner(file); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is scanner the best way to read file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, do you have a suggestion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be buffered reader but not sure if there are any new libraries which can be more better. Please check.
Alright!, then you suggest an input box only for the param, I will take this usecase, it is fine? |
ok, can you please check if this use-case is feasible or not? You need to do a little more research on this. |
Hi @preetkaran20 sorry for the delay, I did a research about this use-case, is feasible, it would be the same idea as this scenario but with an input for the images: It occurred to me an use case where the user can change the file name of an image, for example: an user can select one image with a select input, once selected he can write in an input text the new name of the image, when the user press submit the app will show the image with a successful message. This will be the "normal" case. In the other case, the user write a new name with a path like: "/etc/passwd", and when the user press submit the app will show the passwd info. What do you think about this approach? |
Yeah, this use case looks good however it is not very clear to me. Can you please elaborate more? Also if possible let's connect over a call to discuss? I work in IST time zone from 10 AM IST to 10 PM IST. Please let me know your preferred timings. |
Hello @preetkaran20 I am so sorry, it seems that for now I will not have enough time to continue with this, I will probably be available in the future but at the moment it is complicated. |
Hi @Ivan12273, No problem. No need to close the PR, we will pick this issue from here only. Thanks a lot for the contribution. thanks, |
Hi!, I was working on the issue #286 I added the Local File Inclusion to the project:
It has two level:
It works similar to Remote File Inclusion, when the user find the folder with the password file it will return the following:
When the users clicks on the hyperlink they will go to the next level:
The users can see the secret data if they successfully performs the vulnerability:
I also made some fixes, these hyperlinks were sending you to broken links when clicking on them, so I added "/ VulnerableApp" to fix them.
And the GitHub icon wasn't on the project so I added it too:
Greetings, I hope these changes are ok :)