Hacker News new | ask | show | jobs
by pilif 4131 days ago
> return ( (Issuer.ToLower().Contains("superfish, inc")) || (IssuerName.ToLower().Contains("superfish, inc")) );

While in this case, it might be ok, please never do this in your own programs. Before deciding to act on something, make sure that you are as precise as possible before taking action.

In this case, as all machines had the same certificate, use the key fingerprint or the whole certificate for comparison. And failing that, do an equality match on the name. A case insensitive substring match is way too wide and you might be accidentally removing things you didn't want to remove ("pilif's Superfish, Including production" is an issuer name of a certificate that would be removed by Lenovo's code).

It's easy to be accurate when checking. It's hard to undo accidental damage. And no matter how much time it takes you right now to go the extra length, it will pale in comparison to the hell you will have to go through once the accident happens.

1 comments

Let's also not forget the multiple 100+ line methods and try-catches which don't even bother to log or handle the error they catch.

There also doesn't appear to be tests, at least at first glance.

Edit... Taking a closer look there is clear copy-pasta and several potential bugs