Conversation
… refine Docker images and build processes.
…oints to host-side type handlers, adding `bleach` for output sanitization.
…dding new language-specific pipe types with their configuration
…uild process with enhanced output formatting
|
Aaah, I thought someone had actually balls to rewrite evaluation code inside kelvin. And this is just docker stuff :D 🥲 |
This isn't just Docker stuff, check type_handlers.py 😅 |
I see, it was hidden as there was many lines. |
| if len(executables) == 0: | ||
| raise BuildError("No executable has been built.", logs=html_output) | ||
| elif len(executables) > 1: | ||
| raise BuildError(f"Multiple executables have been built: {','.join(executables)}", logs=html_output) |
There was a problem hiding this comment.
I did not yet go through the entirety of this, but from what I can tell, this leads to XSS if a student can set the filename of the executables (for example with the <img src=x trick) because neither this raise, nor the catch sanitizes the executable names. When I attempted it on my machine (after removing non-buildable images like dotnet - an && seems to be missing there between apt-get install and apt-get clean - and adding support for make to gcc image, because the make executable was missing from the image and due to the presence of cmake support, I doubt this is intentional), this XSS worked. It does not work on master branch, because there gcc's entry.py contains a bleach.clean(str(e)) in the except.
There was a problem hiding this comment.
I did not yet go through the entirety of this, but from what I can tell, this leads to XSS if a student can set the filename of the executables (for example with the
<img src=xtrick) because neither this raise, nor the catch sanitizes the executable names. When I attempted it on my machine (after removing non-buildable images like dotnet - an&&seems to be missing there betweenapt-get installandapt-get clean- and adding support for make to gcc image, because themakeexecutable was missing from the image and due to the presence of cmake support, I doubt this is intentional), this XSS worked. It does not work on master branch, because there gcc'sentry.pycontains ableach.clean(str(e))in the except.
Hi, thanks for the early review. To be honest, I haven’t started testing the code yet and this is just an early scratch, which is why it’s still in draft. I mainly wanted an estimate of how much I’ve changed so far.
Your point definitely makes sense. I need to re-check and sanitize everything that is input/output. Btw, I’m planning to replace bleach with nh3, since bleach was discontinued in 2023. I’ll definitely ask for your review again, if you’re okay with that, once this is ready (it will probably be split into three PRs).
3 participants
No description provided.