danielsen / NetSPF

.NET SPF tools
MIT License
2 stars 3 forks source link

In a special case, the limit of 10 lookups is not restricted #6

Closed StephanMoeller closed 1 year ago

StephanMoeller commented 2 years ago

If you have the following setup:

mydomain0.com => lookup gives "$"v=spf1 mydomain1.com ~all" mydomain1.com => lookup gives "$"v=spf1 mydomain2.com ~all" mydomain2.com => lookup gives "$"v=spf1 mydomain3.com ~all" mydomain3.com => lookup gives "$"v=spf1 mydomain4.com ~all" mydomain4.com => lookup gives "$"v=spf1 mydomain5.com ~all" mydomain5.com => lookup gives "$"v=spf1 mydomain6.com ~all" mydomain6.com => lookup gives "$"v=spf1 mydomain7.com ~all" mydomain7.com => lookup gives "$"v=spf1 mydomain8.com ~all" mydomain8.com => lookup gives "$"v=spf1 mydomain9.com ~all" mydomain9.com => lookup gives "$"v=spf1 mydomain10.com ~all" mydomain10.com => lookup gives "$"v=spf1 mydomain11.com ~all" mydomain11.com => lookup gives "$"v=spf1 mydomain12.com ~all" mydomain12.com => lookup gives "$"v=spf1 mydomain13.com ~all"

In this case, there is a missing subtraction from RemainingQueries Include.cs just before is calls CheckHost again. Again, my suggestion is to start make things non-static to start having some regression tests that will guard against re-introduced bugs in the future.

StephanMoeller commented 2 years ago

In Include.cs, adding this:

            if (SpfStatement.RemainingQueries-- <= 0)
                throw new Exception("DNS Lookup maximum reached.");

Just before calling spfResolver.CheckHost(...) seems to fix the issue. This fixes both this and the eternal loop issue.

jol64 commented 1 year ago

I can confirm both the issue and that the fix works as advertised. However due to the convention imposed by the exception handling the added line should go outside of try/catch. I put it at line 20.