WordPress.org

Making WordPress.org

Opened 6 years ago

Closed 6 years ago

#286 closed enhancement (wontfix)

Setup SVN pre-commit hooks to enforce plugin guidelines

Reported by: iandunn Owned by:
Milestone: Priority: normal
Component: Plugin Directory Keywords:
Cc:

Description

@raanan shared an article about 3rd parties purchasing Chrome extensions in bad faith and then adding inappropriate ads without informing the users, and asked if the WordPress.org repository was vulnerable to this.

That made me wonder about setting up a pre-commit hook to scan for malware (assuming there's a good ope-source malware database to check commits against). That would also protect against situations where a developer's account is compromised.

It also made me wonder if we couldn't also automatically enforce some of the plugin guidelines.

  • Embedding offsite images/etc
  • Missing License header
  • More than 12 terms in Tags header
  • Commit frequency
  • Capital P

There could be some situations that are suspicious but not always inappropriate, so we might want to send an e-mail to the Plugin Review Team, but not automatically block the commit.

  • Calls to base64_decode() or eval() could be obfuscated code or malware.

Is it worth the effort? Is there a good way to gracefully handle false positives?

Change History (2)

#1 @Otto42
6 years ago

We actually already have a commit hook to scan for certain types of things like eval and base64 and such. It emails me, @nacin, and @duck_, I believe.

The number of false positives is so large as to almost make it not worth the effort. Almost. I've found a few things there, but most of it is fine. Take a simple "base64" example. There exist APIs which actually use variants of base64 in them (Facebook, for one), and thus it's not malware all the time. And actual malware is better at hiding "base64" calls than you would believe. We actually have better luck at scanning for gibberish which looks like encoded code.

As for people purchasing plugins and then adding "stuff" to them, this has already occurred several times. In a few cases, we've removed them because of said stuff, and in other cases, the stuff was not particularly objectionable. In at least one case, the problem solved itself by somebody taking the previous code and forking it to a new name and developing it further separately.

Realistically, adding a pre-commit hook isn't at all difficult, but just you try writing code to scan for those sort of things and see how well you do at it. It ain't easy. Code is not amenable to automated scanning for intent.

#2 @iandunn
6 years ago

  • Resolution set to wontfix
  • Status changed from new to closed

Sounds good, thanks for the thorough explanation :)

Note: See TracTickets for help on using tickets.