Page MenuHomeMiraheze
Create Task
Maniphest T6525

Recommending R4356th for security reviewer
Closed, ResolvedPublic
Actions

Description

After discussing with @R4356th about his php and other programming experience, he told me he's actually written a MediaWiki extension, and has extensive php programming knowledge and experience. Additionally, he has knowledge of and experience working with and reviewing code for potential security vulnerabilities (including, but not limited to, SQL injections). So, after a brief conversation, I recommended he apply to volunteer as a security reviewer. As I understand it, @Southparkfan will likely review the request, recommend he review an existing pending extension in the queue, and provide some sort of analysis. After that, @Southparkfan, together with any other Site Reliability Engineering team members, will review the request and make a determination.

At any rate, at this point, I'm going to turn it over to @R4356th to discuss in more detail his relevant experience.

Related Objects

Event Timeline

Dmehus triaged this task as Normal priority.Nov 29 2020, 18:41
Dmehus created this task.
Herald added subscribers: Universal_Omega, RhinosF1, Reception123. · View Herald TranscriptNov 29 2020, 18:41
Dmehus updated the task description. (Show Details)Nov 29 2020, 18:42
R4356th added a comment.Nov 29 2020, 19:15

Hello,

I have knowledge of PHP, XSS and SQL injection. I have worked on a MediaWiki extension (it has been approved and installed on Miraheze) and it could be an example of my work. Though not mentioned in the Vacancies page as a requirement, I also have knowledge of JavaScript and Python. Reading the previous requests for this position, I presume I am supposed to review an extension currently waiting to be reviewed. If that is the case, I am ready to do that. Thank you.

Universal_Omega awarded a token.Nov 29 2020, 20:04
John added a subscriber: John.Dec 22 2020, 13:13

This seems to have had no follow up at all from @Southparkfan. To minimise the risk of it being delayed even more, @R4356th can you pick an extension to review from the Needs Review column of the Extensions workboard, provide your comment on that task, and then link to those comments on this task please to allow us to process the request further. Thanks!

John assigned this task to R4356th.Dec 22 2020, 13:13
R4356th added a comment.Dec 22 2020, 15:30

T6621 seems to require BlueSpice. I will review another extension.

R4356th added a comment.Dec 23 2020, 13:03

Apologies for the delay. T6531 also requires BlueSpice. I reviewed and left a comment (T6607#130140) on T6607. Do I need to do anything else? Thank you.

Universal_Omega reassigned this task from R4356th to Southparkfan.Sun, Jan 3, 05:40

Reassigning to Southparkfan, for how to proceed here since there are no more extensions to review.

Southparkfan reassigned this task from Southparkfan to R4356th.Fri, Jan 8, 20:08

@R4356th your review in T6607 looks good. Since you maintain EmbedSnap, I have decided to review that extension as well.

EmbedSnap takes care of using settype (which is good) and uses ENT_QUOTES in htmlspecialchars (which is even better), but does not rely on MediaWiki functions for validating and sanitising input. This makes it harder to review extensions and increases the chance of forgetting to sanitise one input that will be responsible for another XSS vulnerability. Using functions that are safe by default reduces reviewer anxiety: everyone should defer sanitising input to MediaWiki. https://www.mediawiki.org/wiki/Security_checklist_for_developers is a good checklist for improvements!

I have a few questions left. There are no 'wrong' answers here ("I don't have experience with this" is a honest and valid answer), just interested.

  1. Are you experienced with the OWASP Top Ten?
  2. If you skim through https://www.mediawiki.org/wiki/Manual:MediaWiki_Security_Guide, do you see anything you don't understand?
  3. Are you familiar with the https://www.mediawiki.org/wiki/Manual:MediaWiki_Developer%27s_Guide?
  4. Do you maintain more extensions than EmbedSnap? Can you point me to more open source projects you have contributed code for? Or are you a bug bounty hunter by night?
  5. What is your strategy for reviewing extensions?
    1. I look at the code with a checklist next to me
    2. I use Vagrant or Docker to spin up a MediaWiki instance and pentest the whole thing
    3. I use specific tools to analyse the code for vulnerabilities or find vulnerabilities in web applications
    4. (etc.)
  6. Are you familiar with security assessments?
  7. Currently, Miraheze has no requirements for security reviewers. Extensions are approved or rejected at the discretion of reviewers. Could you help with formalising the review process?
Naleksuh removed R4356th as the assignee of this task.Sat, Jan 9, 06:18
Naleksuh assigned this task to R4356th.
Naleksuh added a subscriber: Naleksuh.
R4356th reassigned this task from R4356th to Southparkfan.Sat, Jan 9, 13:35

EmbedSnap takes care of using settype (which is good) and uses ENT_QUOTES in htmlspecialchars (which is even better), but does not rely on MediaWiki functions for validating and sanitising input. This makes it harder to review extensions and increases the chance of forgetting to sanitise one input that will be responsible for another XSS vulnerability. Using functions that are safe by default reduces reviewer anxiety: everyone should defer sanitising input to MediaWiki. > https://www.mediawiki.org/wiki/Security_checklist_for_developers is a good checklist for improvements!

I am aware of the checklist, and using MediaWiki functions, by which you indicated the HTML class, I assume, is planned.

Are you experienced with the OWASP Top Ten?

If experienced is supposed to mean looking in others' code potentially for these vulnerabilities then no. Otherwise, if you mean having knowledge of them then yes.

If you skim through https://www.mediawiki.org/wiki/Manual:MediaWiki_Security_Guide, do you see anything you don't understand?

No, I do not.

Are you familiar with the https://www.mediawiki.org/wiki/Manual:MediaWiki_Developer%27s_Guide?

Yes, I am.

Do you maintain more extensions than EmbedSnap? Can you point me to more open source projects you have contributed code for? Or are you a bug bounty hunter by night?

No, but I am working on one which I hope to make open source within a few hours. Yes, I have contributed to many open source projects but those may not be what you are looking for as most are in JavaScript. Most of my open source PHP work is related to MediaWiki extensions. The latter does not apply to me.

What is your strategy for reviewing extensions?

I read the code with the security checklist for developers next to me. After I am done reading the whole code (or sometimes right after I notice something that makes it look vulnerable or useless to me), I use a local instance of MediaWiki to pentest it.

Are you familiar with security assessments?

Yes, I believe so.

Currently, Miraheze has no requirements for security reviewers. Extensions are approved or rejected at the discretion of reviewers. Could you help with formalising the review process?

Yes, I hope to be able to and would be willing to answer any questions you may have regarding this.

Southparkfan reassigned this task from Southparkfan to Universal_Omega.Sun, Jan 24, 16:30

Per discussion.

Herald added a project: Universal Omega. · View Herald TranscriptSun, Jan 24, 16:30
Universal_Omega added a comment.Sun, Jan 24, 16:37

@R4356th this all looks good. Approved.

  • When in doubt, ask.
  • When not 100% positive don't approve.

Once you give confirmation you have 2 factor authentication enabled on Phabricator, you can be added to acl*security_reviewers

R4356th added a comment.Sun, Jan 24, 16:40

Yes, I have 2FA enabled.

Universal_Omega closed this task as Resolved.Sun, Jan 24, 17:50

Done.

R4356th awarded a token.Sun, Jan 24, 18:05
Dmehus awarded a token.Sun, Jan 24, 21:26