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.

Sign up for GitHub

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

Jump to bottom

Notes aren't properly escaped in Python, security concern #3

Closed
Burrito-Bazooka opened this issue Oct 15, 2016 · 1 comment
Closed

Notes aren't properly escaped in Python, security concern #3

Burrito-Bazooka opened this issue Oct 15, 2016 · 1 comment

Comments

@Burrito-Bazooka
Copy link

Burrito-Bazooka commented Oct 15, 2016 •
edited
Loading

Among the active forks of ulogme, yours is the only one that claims ulogme is secure.

Please see: karpathy#48

Sidenote: @karpathy released this under MIT first, so I'd suggest you add credit to him wherever you have credited yourself as well? Unless you have an agreement with him?
Burrito-Bazooka added a commit to Burrito-Bazooka/ulogme that referenced this issue Oct 16, 2016
@Burrito-Bazooka
fixes Naereen#3
6f0f8fd
Naereen added a commit that referenced this issue Oct 16, 2016
@Naereen
Thanks for @Burrito-Bazooka for #3.
97e3135
Naereen added a commit that referenced this issue Oct 16, 2016
@Naereen
Improvements on ulogme_serve.py:
c7cca4f 
- Better handling of Ctrl+C or ^C (KeyboardInterrupt)
- Better warning message if the PORT was already used
- Better exceptions message
- Cleaner organization: address and port and chdir are in the __file__ = '__main__' if case
- Security concern: karpathy#48 suggested to use IP='127.0.0.1' and not ''
- Added the writenote(..) function to improve (fix?) security threats as indicated in #3
@Naereen
Copy link
Owner

Naereen commented Oct 16, 2016

Hi,

Thanks for your remarks.
I forgot to mention any other contributors, sorry (I copy-pasted a piece of Markdown in the README.md, without remembering it was a fork)... That's fixed (97e3135).

About the security, I changed my claimed, and I tested and merged your suggestion (karpathy#48), cf. (c7cca4f). Thanks!

@Naereen Naereen closed this as completed Oct 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Burrito-Bazooka @Naereen