thedevs-network / kutt

Free Modern URL Shortener.
https://kutt.it
MIT License
8.41k stars 1.09k forks source link

Use more resources for checking source url than Safe Browsing - e.g. DNSBL #252

Open CMiksche opened 4 years ago

CMiksche commented 4 years ago

I recently added the issue #251. I think problems like this (getting listed on a blacklist) could be prevented if this project would use more checks on the source url which should be shorted.

After looking at the source code (https://github.com/thedevs-network/kutt/blob/develop/server/controllers/validateBodyController.ts), this project currently only seems to use Google Safe Browsing API. However, there are more blacklists which could be easily checked.

We could use something like https://github.com/hassansin/node-dnsbl-lookup to check the source Domain and IP on multiple blacklists and prevent malicious sites from using the url shortener.

I maybe could create a PR for this, but i wanna see more opinions on this first ...

poeti8 commented 4 years ago

Yes this is problem I have been dealing with a lot.

Adding new checks is fine but to make sure it's not slowing down the link creation, we can have a queue and check recently created links one by one and ban the problematic ones.

CMiksche commented 4 years ago

Adding new checks is fine but to make sure it's not slowing down the link creation, we can have a queue and check recently created links one by one and ban the problematic ones.

So you are suggesting that the user can submit a link, go away and post it in some community and don't sees that the link afterwards get banned / deleted?

I don't think thats a good idea - i would check it in the link creation process.

I already tried to integrate https://github.com/hassansin/node-dnsbl-lookup but haven't succeed because dnsbl-lookup uses EventEmitter and you use Async/await. https://www.npmjs.com/package/dnsbl also uses Async/await - so that would be easier to integrate, but in dnsbl you have to manually enter the blacklists in which the domain / ip should be checked ...

Btw: why do you use Async / await so much? I am used to RxJS / Events and really like the idea behind RxJS.

poeti8 commented 4 years ago

That was the idea if we had to add more API calls, if we're only going to add one then, yeah, we can call in the link creation process.

Yes, I'd prefer async/await, if not available can write a wrapper around it to turn it into promise.

trgwii commented 4 years ago

I already tried to integrate https://github.com/hassansin/node-dnsbl-lookup but haven't succeed because dnsbl-lookup uses EventEmitter and you use Async/await. Btw: why do you use Async / await so much? I am used to RxJS / Events and really like the idea behind RxJS.

@CMiksche

Events / Streams aren't in any way incompatible with async / await, in fact they work really well together, you have perhaps seen the new-ish for..await..of loop?

for await (const item of stream) {
  // ...
}