I am not that familiar with the System.Threading.Timer class, but should it not be disposed before being replaced? Since dueTime and period are specified in the call to Change, it looks like ValidateServerSig should be called every 15 seconds (starting 15s after the call to change) and since ValidateServerSig always sets up a new timer when it's called - without ever disposing an existing timer - that could lead to infinite timers as long as the result is true?

Checking for response.Failed after result.Valid should be useless, because the previous check should have ended the method call. I suppose the real check should have been for !result.Valid?

Look like the issue with a timer life-cycle. It's being recreated each time. The old instance of timer is not disposed - thus it may fire for a long time (until garbage collected). Additionally the timer is configured to run many times. In this case it should be configured to run once nextcheck.Change(TimeSpan.FromSeconds(15), TimeSpan.FromMilliseconds(-1));

The biggest issue with this code is that ValidateServerSig creates a new timer object to call itself again every 15 seconds. So the initial call will setup the timer, then 15 seconds later it will setup a new timer, then 15 seconds later it will setup 2 new timers (since both the old timers fire), then 15 seconds later it will setup 4 new timers and so on. The system will run out of some sort of resource, possibly network, or outbound ports, or timer kernel objects. CPU and memory will fall as well over since this is an exponential failure, however other failures might prevent the exponential nature of the bug taking full effect. The remote server will be getting dos'd quite nicely.

If the signature is retrieved successfully but is invalid, the method will nonetheless return true, as the conditional logic around result.valid is way off. An attacker may trivially impersonate the remote server.

It looks like the bug is that, after the first call to "ValidateServerSig()", there is no checking of the return value for that function. Thus, each time the timer kicks in and executes "ValidateServerSig()", it's a waste of time become nothing is acting on it's return value.

To add to my comment - since nothing checks for "false" after the first time, "ValidateServerSig()" will continue to execute, causing a degrade in system resources.

1. Adding a new timer every 15 sec

2. Error in KeySignatureMatches result validation

3. ServerSigFailed event can be called but nobody will be notified

It will return true, even though CheckPublicKeySignatureMatches result is not Valid. The whole "if(response.Failed)..." will never be called after "if(result.Valid)" since it has already been checked. Unless of course CheckPublicKeySignatureMatches changes "response" which would be idiotic...

One bug not mentioned so far is that timer ticks can be concurrent.

Also there should not be a timer at all. A sleep loop would be much simpler.