Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security SQL: final queries #1534

Merged
merged 9 commits into from
Nov 24, 2020
Merged

Security SQL: final queries #1534

merged 9 commits into from
Nov 24, 2020

Conversation

tomvangoethem
Copy link
Contributor

@tomvangoethem tomvangoethem commented Nov 16, 2020

Progress on #906

@tomvangoethem
Copy link
Contributor Author

I'm guessing that the failed checks are unrelated to this particular pull request? The reported error is:

Error: Unable to process command '::add-path::/opt/hostedtoolcache/node/12.19.0/x64/bin' successfully.
Error: The `add-path` command is disabled. Please upgrade to using Environment Files or opt into unsecure command execution by setting the `ACTIONS_ALLOW_UNSECURE_COMMANDS` environment variable to `true`. For more information see: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/

@tunetheweb
Copy link
Member

Just merged a fix (#1535) into main. Can you update your branch?

@rviscomi rviscomi added analysis Querying the dataset ASAP This issue is blocking progress labels Nov 16, 2020
@rviscomi rviscomi added this to the 2020 Analysis milestone Nov 16, 2020
@tomvangoethem
Copy link
Contributor Author

Yep, that fixed it 👍

Co-authored-by: Barry Pollard <barry_pollard@hotmail.com>
@rviscomi rviscomi marked this pull request as ready for review November 22, 2020 18:17
@rviscomi
Copy link
Member

@tomvangoethem please see @bazzadp's feedback. We should merge this ASAP.

@tomvangoethem
Copy link
Contributor Author

Will fix it tomorrow!

@tomvangoethem
Copy link
Contributor Author

Updated cryptominer query. However, as @bazzadp mentioned, the number looks very low. Looking into that right now.

@tomvangoethem
Copy link
Contributor Author

Coinhive: shut down
CoinImp: potentially outdated URL pattern in Wappalyzer. Based on the documentation, it should be ^https://www\.hostingcloud\.racing/.+\.(js|wasm) - I get 167 pages with this for desktop; Wappalyzer reports 175 (sets could be mutually exclusive though)
WebMinePool: some 67 desktop pages include https://webminepool.com/lib/base.js - not considered by Wappalyzer
JSECoin: shut down in April 2020

From the paper "Is Cryptojacking Dead after Coinhive Shutdown?":

The results revealed that 99% of sites no longer continue cryptojacking.

So it seems that cryptojacking is not financially viable any more (because of hard fork that caused a 50+% drop in hash rate, and the drop in value of XMR), which makes me think that the results are more or less representative

Copy link
Member

@tunetheweb tunetheweb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. A few very minor nits.

This good to merge?

@tomvangoethem
Copy link
Contributor Author

Yep, good to merge I think. That should be it for Security SQL 🥳

@tunetheweb
Copy link
Member

Great stuff. Now just to write the thing 😁

@tunetheweb tunetheweb merged commit d244a5e into main Nov 24, 2020
@tunetheweb tunetheweb deleted the security-sql-2020 branch November 24, 2020 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Querying the dataset ASAP This issue is blocking progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants