navossoc / KeePass-Yet-Another-Favicon-Downloader

Yet Another Favicon Downloader for KeePass 2.x
MIT License
532 stars 29 forks source link

Direct/Alternate icon field on entries? #3

Closed WeaverThree closed 6 years ago

WeaverThree commented 7 years ago

Right now I have gamersgate, where I know that https://www.gamersgate.com/favicon.ico exists but the program won't pick it up for some reason. It'd be great if I could add a custom field called FaviconSource to the entry and have it just attempt to download the file there.

Another custom field named something like FaviconSite could be used for when you have a situation where you want the site url to be 'accounts.blah.com' but you want it to take the favicon from 'blah.net' or wherever, just substitute the url and otherwise do the normal thing

navossoc commented 7 years ago

It should redirect without any help, but probably this is one of the edge cases and may be some bugs there.

Which is exactly URL are you entering? What operating system are you using? http://gamersgate.com; http://www.gamersgate.com; https://gamersgate.com; https://www.gamersgate.com; or something else?

Let me know the URL and I'll look into.

navossoc commented 7 years ago

Well, I did a quick lookup. The response error is this: The server committed a protocol violation. Section=ResponseHeader Detail=CR must be followed by LF

You should read the entire answer before doing something.

There is a workaround on Windows for it (not sure about mono).

Locate the file: C:\Program Files (x86)\KeePass Password Safe 2\KeePass.exe.config

You need to add this inside the <configuration> tag

<system.net>
    <settings>
        <httpWebRequest useUnsafeHeaderParsing="true" />
    </settings>
</system.net>   

It should look like this:

<?xml version="1.0" encoding="utf-8" ?>
<configuration>
    <startup>
        <supportedRuntime version="v4.0" />
        <supportedRuntime version="v2.0.50727" />
    </startup>
    <runtime>
        <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">
            <dependentAssembly>
                <assemblyIdentity name="KeePass"
                    publicKeyToken="fed2ed7716aecf5c"
                    culture="neutral" />
                <bindingRedirect oldVersion="2.0.9.0-2.36.0.0"
                    newVersion="2.36.0.17315" />
            </dependentAssembly>
        </assemblyBinding>
        <enforceFIPSPolicy enabled="false" />
        <loadFromRemoteSources enabled="true" />
    </runtime>
    <appSettings>
        <add key="EnableWindowsFormsHighDpiAutoResizing" value="true" />
    </appSettings>
    <system.net>
        <settings>
            <httpWebRequest useUnsafeHeaderParsing="true" />
        </settings>
    </system.net>   
</configuration>

This workaround may lead to security issues. If you trust the server, this may not be a problem.

For more information you should read this: HttpWebRequestElement.UseUnsafeHeaderParsing

I'm not sure how many broken HTTP servers are out there.

If it is possible to change this setting at run time, I can add a custom setting inside KeePass to allow/deny this behavior. Of course the default value would be UseUnsafeHeaderParsing = false.

However, I'm not sure if it's worth sacrificing security for a simple favicon.

[]'s

photonometric commented 6 years ago

I don't know code, so I'm not sure if there's something significant and obvious in that "unsafe header parsing." The link seems to suggest it can throw back some kind of unsafe or damaging or malicious code if you create that WebException error.

However....that's talking about a web browser that needs to be flexible enough to handle however a web page is configured, or misconfigured. This may be naive, but if the only possible response you will accept is a tiny favicon image, wouldn't it be fairly easy to "firewall" (so to speak) any kind of webexception reply that might cause a security issue? It seems like it should be possible, but it might mean sacrificing the accuracy of the logged failure, which doesn't seem like a big deal to me...there are other ways to diagnose favicon problems; I think this great plugin should concentrate on grabbing them and making things like notifications and logs secondary.

btw, I'm not sure if that website changed in the past two months, but Firefox 54 opens that favicon with no trouble or warnings at all, while this plugin still returns an error...

navossoc commented 6 years ago

@photonometric You are right, the documentation lacks details about it, so we will never know exactly what that is all about. Maybe they're overreacting ...

About the "firewall" idea, I agree, probably shouldn't be that hard, but we must be careful. Since I released the plugin I already have plans to do a major rewrite to improve the parsing/downloading code. After that I think it is possible to easily handle this kind of case without compromising security. Since the others plugins (Favicon Downloader and Custom Icon Dashboarder) also can't download and we want to keep our passwords secure, I think it's best to leave the way it is for now.

(Better safe than sorry, right?)

Yes, the site works fine in any browser, now and back then. The browsers seems to be less strict with the specifications and still secure(?).