You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zeppelin.apache.org by GitBox <gi...@apache.org> on 2021/09/01 14:47:50 UTC
[GitHub] [zeppelin] izeren-amzn opened a new pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
izeren-amzn opened a new pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212
### What is this PR for?
Anchor links that launch new tabs using target="_blank" are vulnerable to tab nabbing
see: https://owasp.org/www-community/attacks/Reverse_Tabnabbing
### What type of PR is it?
Improvement
### Todos
* Add rel="noopener noreferrer" to the anchor links (https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#tabnabbing)
### What is the Jira issue?
https://issues.apache.org/jira/browse/ZEPPELIN-5395
### How should this be tested?
Child pages from opened links should not contain referrer info or links to the parent one
### Screenshots (if appropriate)
### Questions:
* Does the licenses files need update?
No
* Is there breaking changes for older versions?
Content of parent pages will no longer be accessed with back referencing from the child ones
* Does this needs documentation?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914337983
> So, I can remove isPlatform check to support simplicity
Yes, please remove this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-916858321
> @izeren-amzn
> If you are good at front-end coding, the Zeppelin project would be very grateful if you could finish the new web front-end.
> https://issues.apache.org/jira/browse/ZEPPELIN-4321
I will have a look if I can do anything for this issue but I am not that experienced at front-end coding
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914332151
> Do you need isPlatformBrowser because of the tests?
> Do we need the changes in `zeppelin-web-angular/src/app/share/about-zeppelin/about-zeppelin.component.html` and `zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts`? I thought that `ExternalLinkDirective` applies to all links.
I put it here just in case of some future SSR, it is not necessary right now.
`component.html & component.ts`: my bad, missed to revert these files, we don't have to do manual fixes for it.
So, I can remove isPlatform check to support simplicity
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914222922
Do you need isPlatformBrowser because of the tests?
Do we need the changes in `zeppelin-web-angular/src/app/share/about-zeppelin/about-zeppelin.component.html` and `zeppelin-web-angular/src/app/pages/workspace/notebook/paragraph/paragraph.component.ts`? I thought that `ExternalLinkDirective` applies to all links.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914202516
> > I have applied this fix for all the links apart from the angular rendered. For angular rendered links the more general solution can be applied (https://coryrylan.com/blog/managing-external-links-safely-in-angular), but before I will do this workaround, I would like to confirm it with the maintainers
>
> I would prefer a general solution for Angular. I took a look at the blog post and it seems that the solution works without third party dependencies.
>
> I can confirm that you added the `rel="noopener noreferrer"` to all other links (except in zeppelin-web-angular) with `target="_blank"`.
I have added this solution to angular based links and I have run tests locally accordingly to https://zeppelin.apache.org/contribution/contributions.html
I also have started zeppelin locally to check a couple of angular generated links in firefox and chrome, so it is ready to review and PR checks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-913746575
Hi @zjffdu, since Philipp is busy, could you please have a look at this PR?
Short summary:
There is an issue with vulnerability caused be links with target="_blank"
https://owasp.org/www-community/attacks/Reverse_Tabnabbing
It can be addressed by adding rel="noopener noreferrer" to the links
https://cheatsheetseries.owasp.org/cheatsheets/HTML5_Security_Cheat_Sheet.html#tabnabbing
I have applied this fix for all the links apart from the angular rendered. For angular rendered links the more general solution can be applied (https://coryrylan.com/blog/managing-external-links-safely-in-angular), but before I will do this workaround, I would like to confirm it with the maintainers
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-910511904
> Thank you. Changes LGTM.
>
> Just wondering why the addition of `noreferrer` `noopener` was not added for the following:
>
> * `docs/interpreter/cassandra.md`... lines 166, 175, 184 (just wondering since `docs/interpreter/jdbc.md` has been treated)
> * `zeppelin-web/src/app/home/home.html`... lines 60, 67, 69, 71 (is the reason that the links are to `github.com` and `apache.org` and we definitely trust those websites forever?
> * `zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html`... lines 98, 328 (is it because an Angular plugin is used to automatically add these tags at transpile time?)
> * and a few other minor similar examples that broadly follow the above 3 categories...
It seems I have missed a few files. Added more fixed links, thank you
There are still some anchors rendered with angular (zeppelin-web-angular). Probably, it is good idea to add special directive for any external links https://coryrylan.com/blog/managing-external-links-safely-in-angular rather than fix all that links manually. But I would rather ask maintainers before
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914048005
> I have applied this fix for all the links apart from the angular rendered. For angular rendered links the more general solution can be applied (https://coryrylan.com/blog/managing-external-links-safely-in-angular), but before I will do this workaround, I would like to confirm it with the maintainers
I would prefer a general solution for Angular. I took a look at the blog post and it seems that the solution works without third party dependencies.
I can confirm that you added the `rel="noopener noreferrer"` to all other links (except in zeppelin-web-angular) with `target="_blank"`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-916851656
@izeren-amzn
If you are good at front-end coding, the Zeppelin project would be very grateful if you could finish the new web front-end.
https://issues.apache.org/jira/browse/ZEPPELIN-4321
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-915182573
I have taken a look at the failed tests:
Tests in error:
ZeppelinIT.testSparkInterpreterDependencyLoading:221->AbstractZeppelinIT.clickAndWait:164 » ElementNotInteractable
ZeppelinIT.deleteTrashNode:347->AbstractZeppelinIT.deleteTrashNotebook:153 » NoSuchElement
@Reamer, it seems, that these tests are not related to my commits. And I have seen the same errors in the other PRs. So what are the next steps before merge should be taken?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-914339995
> > So, I can remove isPlatform check to support simplicity
>
> Yes, please remove this.
I have already removed it in the last commit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] Reamer commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
Reamer commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-915217247
> @Reamer, it seems, that these tests are not related to my commits. And I have seen the same errors in the other PRs. So what are the next steps before merge should be taken?
No further steps. We are just waiting a bit for possible comments from other maintainers.
I will merge this PR on Friday (10/09/2021) if no further comments are received.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-915121445
> > So, I can remove isPlatform check to support simplicity
>
> Yes, please remove this.
I have run tests from contribution guide. It seems that PR checks could be started
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] CrynetLogistics commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
CrynetLogistics commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-910439230
Thank you. Changes LGTM.
Just wondering why the addition of `noreferrer` `noopener` was not added for the following:
* `docs/interpreter/cassandra.md`... lines 166, 175, 184 (just wondering since `docs/interpreter/jdbc.md` has been treated)
* `zeppelin-web/src/app/home/home.html`... lines 60, 67, 69, 71 (is the reason that the links are to `github.com` and `apache.org` and we definitely trust those websites forever?
* `zeppelin-web-angular/src/app/pages/workspace/interpreter/item/item.component.html`... lines 98, 328 (is it because an Angular plugin is used to automatically add these tags at transpile time?)
* and a few other minor similar examples that broadly follow the above 3 categories...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] asfgit closed pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-916843816
> > @Reamer, it seems, that these tests are not related to my commits. And I have seen the same errors in the other PRs. So what are the next steps before merge should be taken?
>
> No further steps. We are just waiting a bit for possible comments from other maintainers.
>
> I will merge this PR on Friday (10/09/2021) if no further comments are received.
It looks like there are no new comments on this issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [zeppelin] izeren-amzn commented on pull request #4212: [ZEPPELIN-5395] Address tab nabbing vulnerability
Posted by GitBox <gi...@apache.org>.
izeren-amzn commented on pull request #4212:
URL: https://github.com/apache/zeppelin/pull/4212#issuecomment-911623063
@Reamer, could you please have a look at the issue? I am not sure if it is okay to add a new angular directive with automated rendering of rel="noopener noreferrer" for all external links, therefore I would like to take an advise from the maintainers. For non-angular html links I added it manually
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@zeppelin.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org