hannob / snallygaster

Tool to scan for secret files on HTTP servers
BSD Zero Clause License
2.07k stars 228 forks source link

Exend svn and add cvs test #73

Open security-companion opened 2 years ago

security-companion commented 2 years ago

I compared the ZAPproxy-plugin for finding hidden files with snallygaster and found some differences. Therefore I created this PR to add some missing checks.

hannob commented 2 years ago

Can you explain the intention behind this?

The svn part looks like another test for svn, just done in a different way.

For CVS: I think I had previously support for this, but removed it because it practically never hit anything and only produced false positives. (Also see the "New Tests" part of https://github.com/hannob/snallygaster/blob/main/CONTRIBUTIONS.md ).

hannob commented 2 years ago

Closing this for now, if you feel this has value please re-open with an explanation why you think these additional tests are valuable (ideally with some data on findings).

security-companion commented 2 years ago

@hannob Sorry for not answering earlier. I didn't know that you had problems with CVS before.

For svn: I added the second check as I found it on zaproxy. Here can be found some more details about svn. I think depending on which version of svn is used one of the 2 checks would trigger.

hannob commented 2 years ago

The link you provided indicates we should be smarter in how to detect svn repos, but it's a bit scarce on details (e.g. not mentioning which versions use which format). But adding another redundant check does not seem to be the way forward.

It's been a while when I wrote this check, it seems snallygaster merely checks that .svn/entries is a number.

Do you have any examples of svn repos in old or new format that aren't detected that we could use to verify this?

For CVS: Unless you have evidence that this is in any way a relevant problem I'd just ignore it. As said, I had checks before and never found anything, I think CVS these days is just not used any more and in the days CVS was used it was unusual to use SCM systems for webpages, so I think the realistic usage of CVS for web roots is very low.

security-companion commented 2 years ago

For me it's okay to remove CVE. I'll remove it from this pull request.

For svn: I installed TortoiseSVN 1.14.3 and checked out a repo. On the disc I could find .svn/entries and .svn/wc.db See screenshot grafik

So if only current versions should be considered one of the 2 checks would be enough.

I'll try with older versions of svn to see how files change on disc.

hannob commented 2 years ago

I'm a bit confused by the oreilly link you provided. In my tests the entries file merely contains a number, and that's what snallygaster tests. The link says it should contain all kinds of things. I didn't find any official documentation from subversion about that file's content.

It's probably worth trying with a variety of old subversion versions to see how the entries file looks and if we need to improve the test, but I currently don't have the time to do that.