Implements front end refresh button

[joaquimds]

Adds an /update action that runs the update command with the provided "name" parameter. Updates are limited by IP to 10 per hour.

Please check for security holes as we are dealing with client input!

[khromov]

Maybe match against the slug in the DB first using a prepared statement? And then if slug exists run the command on the returned slug rather than the user input. I'm not sure it's safe to just pass the name here.

[joaquimds]

Good idea, I've done that now

[khromov]

My general comment is that this is an OK approach. I wonder if we could make a simpler check by just looking at last_fetched and only allowing one fetch every hour or so per plugin. That would be slightly more abuse-friendly but would save some code. Just a thought, like I said the proposed solution is fine as well!

[joaquimds]

I think using the last_fetched wouldn't be quite right, as there are so many plugins it'd still be quite easy for an attacker to overload the server.

The code could do with a tidy, that's for sure, but I just haven't got the time.

[khromov]

@joaquimds You are right!

Suggestion: Add a php bin/cmd clean_ips command to remove IP:s where last_request is older than 24h or similar, otherwise the table is gonna balloon in size.

[joaquimds]

@khromov I've changed how the getRequestCountByIp function works. Now it starts by pruning any entries that are older than one hour. This also means it doesn't need to check the timestamp in the PHP code anymore. It'll mean the update requests are handled slightly slower, but also means we don't need to add another cron job :-)

[khromov]

@joaquimds Cool! Only caveat with this is that writes lock the entire database on SQLite. I think this has improved on SQLite 3 though.* But it might introduce an additional "attack vector" in terms of DDOSing the database with write requests.

• https://www.sqlite.org/lockingv3.html

[joaquimds]

Good catch. I think the limit of 10 requests per hour should prevent that to an extent, though.

EDIT: Oh wait, no it won't!

[khromov]

@joaquimds But the DELETE query runs on each invocation of getRequestCountByIp, so I think it would still lock the table for a short time every time someone runs /update.

[joaquimds]

@khromov I've changed it so that the DELETE only happens when a new entry is added. This means that the table will still be pruned periodically, but attackers can't cause it to be pruned with every request.

[khromov]

@joaquimds Awesome! 💯 🏆 🚢