davtur19 / DotGit

An extension for checking if .git is exposed in visited websites
GNU General Public License v3.0
372 stars 32 forks source link

add "check root" option #11

Open ZanyMonk opened 2 years ago

ZanyMonk commented 2 years ago

Assessment

Current behavior is to check URL root for each HTTP response received by the browser. Problem with this is that we cannot detect exposed .env (for example) if it's not located at the document root of the webserver.

Solution

This commits enables DotGit to check two locations for each HTTP response :

By default, both of these locations are checked.

An option has been introduced to prevent DotGit from checking URL root automatically.

Improvements

Allow to check every directory in the full URL's path (ie. /project/git/foo > /project/git/foo, /project/git, /project, /).

davtur19 commented 2 years ago

Thanks for the PR!

But unfortunately for now I don't think I can accept it, for two reasons:

  1. Currently the options part would not work as nothing was touched in options.js (and I believe other parts are also malfunctioning)
  2. Searching each directory causes various problems, see #8

For the options part I can fix it, it's not a problem, the problem is to test the extension (use about:debugging and check the network tab) and make sure it doesn't cause problems on websites and in the browser. Furthermore, saving all the visited paths seems to me a bad solution in terms of memory and security.

ZanyMonk commented 2 years ago

Thanks for answering my PR this quick !

  1. Currently the options part would not work as nothing was touched in options.js (and I believe other parts are also malfunctioning)

Oops ! I fixed that part.

  1. Searching each directory causes various problems, see git folder not found if on a subpath #8 [..] Furthermore, saving all the visited paths seems to me a bad solution in terms of memory and security.

Good point, memorizing the URL of every single resource was a terrible idea. I added a filter to keep only "directory" URLs (ending with /).

Maybe chrome.history API can be used to keep track of already checked URLs more cleanly ? Even if you don't accept this PR I'd be glad to hear what you think about this idea.

If you want to preserve web browsing experience for average users, the option can be implemented the opposite way : check root by default, check relative only if option is active. A disclaimer could be added to inform the user about the overhead this option implies.

Also feel free to close this PR if the feature does not fit your project at all, no worries !

Edit: force-pushed two times because of forgotten console.log.

davtur19 commented 2 years ago

Why did you remove the request queue? It is necessary in order not to have thousands of requests in parallel, otherwise the browser risks crashing. For example, if on https://browserleaks.com/ip you press the "Run DNS leak test" button, DotGit goes crazy, trying to visit a lot of sites that don't exist.

davtur19 commented 2 years ago

I do not know if you are still interested in this pr, in any case I would recommend saving the visited paths in a separate list from that of the visited domains, as I think it can become quite large in a short time, and it may be necessary to reset it if it exceeds a certain dimension

ZanyMonk commented 2 years ago

The two chained queues seemed a bit overkill to me, as we only need to buffer a single list of URL candidates. Maybe i'm wrong !

I just tried the leak test you told me about, a request is made for each domain targetted by the script, indeed, but it sums up to 58 requests here, without any crash.

You are right, storing current path in addition to root path makes quite a difference in volumetry. We could keep track of memory usage to help the user decide, but reset is already implemented isn't it ? A separate storage is a good idea, and we could make this storage expire. In any case, remember we store only the "directory part" of paths, which eliminates a decent proportion of all paths (ie. /page/1, /page/2 and /page/3 should search for / and /page/ only). I'll use the extension for some time on a daily basis and to grasp its memory consumption.

I'll have a try implementing some memory management system.

davtur19 commented 2 years ago

I created the queue that way, because before the queue was handled practically in parallel, since js is async. In this way, I am sure that the requests are made one after the other without ever having multiple requests at the same time.

Regarding the leak test, I remember that in the past it gave me problems... Maybe now the queue is no longer needed, I'll do some tests.

For memory management, a separate button could be created to reset paths only... With an indicator of how big the "database" is (?)

davtur19 commented 2 years ago

Sorry for the late reply, I just tested the changes and noticed there is a recursion problem. When the /.git/ folder is checked, the extension tries to check if /.git/.git/ is inside the /.git/ folder you just visited. And it goes in an infinite loop (tested on firefox)

davtur19 commented 2 years ago

I fixed the problem of the loop on firefox, but I noticed that there are also other problems on firefox, for example there are errors in the console and the memory used is not shown