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

unable to add images to register questions #3383

Closed
aasjpvm opened this issue Aug 8, 2018 · 15 comments · Fixed by #3408
Closed

unable to add images to register questions #3383

aasjpvm opened this issue Aug 8, 2018 · 15 comments · Fixed by #3408
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction t:regression Type: Regression. Degraded functionality after update
Milestone

Comments

@aasjpvm
Copy link

aasjpvm commented Aug 8, 2018

After mybb 1815 release, the security questions enable to add html, I use it to add images in the questions, if it was by security reasons, will be a good feature to add bbcode support instead of html, to support the image questions:

Before 1815:
screenshot_10

After 1815:
screenshot_8

The code added was:
screenshot_11

@Eldenroot
Copy link
Contributor

It is not a regression at all... it is safer right now after this change... anyway I see your point, maybe we can find another way.

@aasjpvm
Copy link
Author

aasjpvm commented Aug 9, 2018

The easiest way that I see is to add a parser to render bbcode like with the normal post, it permit more dynamic in this option, like add images, different font type/size/position/weight.
I can collaborate making that change and posting here if you want, it add the same properties that I need without resting the security that was added with the version change mentioned.

@Eldenroot
Copy link
Contributor

Feel free to open a pull request

@euantorano
Copy link
Member

Using the parser would probably be the best option, the other option would be my_strip_tags.

@Eldenroot
Copy link
Contributor

Eldenroot commented Aug 18, 2018

@aasjpvm - will you open a PR with fix?

Parser would be the best way how to solve this.

@Eldenroot
Copy link
Contributor

We can use this regular expression:
(?i)<(?!img|/img).*?>

to replace all html and keep img tag
html.replaceAll('(?i)<(?!img|/img).*?>', '');

@MinusPL
Copy link
Contributor

MinusPL commented Aug 21, 2018

https://i.imgur.com/bQyvFcW.png
Seems to be working with parser. Most likely it would be nice to add few settings to ACP to let people choose what kind of parsing in questions should be possible tough, @euantorano what do you think? Add few settings for parser in registration form or just make this options constant and don't let people change them?

@euantorano
Copy link
Member

I'd probably just hard code it. We don't need to support every option:

  • No support for /me (obviously)
  • No support for HTML
  • No highlight
  • Probably no bad word filtering since only admins can use it?

@MinusPL
Copy link
Contributor

MinusPL commented Aug 21, 2018

Yeah, I actually allowed all mycodes and img ones, still considering is video code is actually needed but most likely not. But for people when it is merged it would be nice to say exactly what they can use and what not in questions. For now I'll update my code with things I actually did and maybe soon create pull request.

@laietechie
Copy link

As previously mentioned, only admins can write security questions, why do we need to forbid html? If going with BB Code, why do we need to limit any tags at all?

@MinusPL
Copy link
Contributor

MinusPL commented Aug 21, 2018

Consider iframe as security problem. If you add source from unknow place and it uses something weird, then you also have this on your registration form. But that's my point of view, and that's why I actually tried attaching here parser.

@euantorano
Copy link
Member

The reason for limiting the ones I listed are:

  • /me: No user logged in on register (obviously), so the code can't be used
  • HTML: We blocked HTML which caused this issue in the first place
  • Highlight: Only used for search results
  • Bad word filtering: Only admins can manage security questions. If they wish to cuss/swear, then they probably don't have a beadwork filter in place for it

@MinusPL video code probably isn't needed, but there might be somebody out there with a question like Name the character at 1:32 in the below video or something I guess...

@MinusPL
Copy link
Contributor

MinusPL commented Aug 21, 2018

Then I'll enable it, but looking how it would be parsed, It could completely destroy any spacing in register form, but I'll test it ;)

I'll also allow smilies, when we are giving people a way to actually customize more questions then smilies are needed :D

@dvz
Copy link
Contributor

dvz commented Aug 21, 2018

As previously mentioned, only admins can write security questions, why do we need to forbid html? If going with BB Code, why do we need to limit any tags at all?

see https://community.mybb.com/thread-219334-post-1312214.html#pid1312214; I'd consider HTML parsing for security questions controversial too as some might interpret that simply as a feature. Adding elements like <iframe>s might make sense in some cases as well.

IMO MyCode as a replacement sounds good.

@MinusPL
Copy link
Contributor

MinusPL commented Aug 21, 2018

Also I ran to another issue - simply security questions might require more space in database now - considering that with MyCodes they could become pretty "big" in terms of text data I ran twice to an error saying that my question is too big.

MinusPL added a commit to MinusPL/mybb that referenced this issue Aug 22, 2018
Added parser for security questions at lines 1221 to 1237.
@effone effone added t:bug Type: Bug. An issue causing error / flaw / malfunction b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled labels Aug 23, 2018
@effone effone added this to the 1.8.19 milestone Aug 23, 2018
@dvz dvz added s:review-needed Status: Review Needed. Possible solution submitted and removed s:resolved Status: Resolved. Solution implemented or scheduled labels Sep 4, 2018
@dvz dvz removed this from the 1.8.19 milestone Sep 6, 2018
@dvz dvz added s:resolved Status: Resolved. Solution implemented or scheduled and removed s:review-needed Status: Review Needed. Possible solution submitted labels May 29, 2019
@effone effone added this to the 1.8.21 milestone May 31, 2019
@effone effone added the t:regression Type: Regression. Degraded functionality after update label May 31, 2019
lairdshaw pushed a commit to lairdshaw/mybb that referenced this issue Oct 11, 2021
[Rebased for 1.9 by Laird]

* fix mybb#3383 - unable to add images to register questions

Added parser for security questions at lines 1221 to 1237.

* Bunch of fixes for previous commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b:1.8 Branch: 1.8.x s:resolved Status: Resolved. Solution implemented or scheduled t:bug Type: Bug. An issue causing error / flaw / malfunction t:regression Type: Regression. Degraded functionality after update
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants