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

escape javascript the server name #59

Merged
merged 1 commit into from
Feb 19, 2022
Merged

escape javascript the server name #59

merged 1 commit into from
Feb 19, 2022

Conversation

olamy
Copy link
Member

@olamy olamy commented Feb 19, 2022

Signed-off-by: Olivier Lamy olamy@apache.org

Signed-off-by: Olivier Lamy <olamy@apache.org>
@olamy olamy added the bug label Feb 19, 2022
@gmcdonald gmcdonald self-assigned this Feb 19, 2022
@gmcdonald gmcdonald merged commit 94dcabc into master Feb 19, 2022
@daniel-beck
Copy link
Member

daniel-beck commented Feb 21, 2022

This roughly doubles the number of escape characters on each round-trip of the global configuration form. Plus it makes any API uses other than the UI really weird.

foobar' -> foobar\' -> foobar\\\' -> foobar\\\\\\\'

Not a security issue, just a bug IMO.

@olamy
Copy link
Member Author

olamy commented Feb 21, 2022

Agree but why would you use a quote here? 😀

@daniel-beck
Copy link
Member

True, what kind of weirdo would name a computer something like Daniel's MacBook Pro?

@olamy olamy deleted the SECURITY-2287-2 branch February 22, 2022 01:41
@olamy
Copy link
Member Author

olamy commented Feb 22, 2022

True, what kind of weirdo would name a computer something like Daniel's MacBook Pro?

correct!
personally I would never trust pushing over ssh and using my personal keys to a server with such name!!
anyway I have removed this escaping #61

but you should note there are some validations on the name added 11yo ago 4280f00 which warn about using some characters Cannot contain < & ' " \ that's coming from using a common validation for all publish-over- plugins and it's defined here https://github.com/jenkinsci/publish-over-plugin/blob/8857c829c4dec430aa945dec3bbb50df2be02b0d/src/main/java/jenkins/plugins/publish_over/BPValidators.java#L47
anyway this validation doesn't prevent to do it which is very useless...

@derline
Copy link

derline commented Jun 22, 2022

True, what kind of weirdo would name a computer something like Daniel's MacBook Pro?

correct! personally I would never trust pushing over ssh and using my personal keys to a server with such name!! anyway I have removed this escaping #61

but you should note there are some validations on the name added 11yo ago 4280f00 which warn about using some characters Cannot contain < & ' " \ that's coming from using a common validation for all publish-over- plugins and it's defined here https://github.com/jenkinsci/publish-over-plugin/blob/8857c829c4dec430aa945dec3bbb50df2be02b0d/src/main/java/jenkins/plugins/publish_over/BPValidators.java#L47 anyway this validation doesn't prevent to do it which is very useless...

When saving Chinese characters,it will become unicode characters with "\",and warn about "Cannot contain < & ' " \".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants