Branch selector disappear immediately (Uncaught EvalError: 'unsafe-eval' is not an allowed)

[CleyFaye]

Description

Hi,

When browsing a repository on gitea, the branch selection widget disappear as soon as the page complete loading.
Disabling JavaScript leave the widget visible.

I'm not sure if any config would be relevant to help diagnose the issue. Although it seems to be fixed in the test instance of gitea, it is currently not available as docker images so the issue persist.

Screenshots

With JavaScript disabled, the widget is visible:

With JavaScript enabled:

Gitea Version

1.16.8 built with GNU Make 4.3, go1.18.2 : bindata, timetzdata, sqlite, sqlite_unlock_notify

Can you reproduce the bug on the Gitea demo site?

No

Operating System

Linux

Browser Version

Chrome 101.0.4951.64

[wxiaoguang]

I know some other users also met the similar problem, I forget the details but in the end it's not a bug.

Could you try to:

1. Check if there is any error message in your browser's console log?
2. Are you using customized templates? Remove customized templates.
3. Visit the page with Private/Incognito Window?

[CleyFaye]

Ah, you found it, somehow.
I initially missed a Content Security Policy error. We indeed have some CSP set which exclude eval() and the like in JavaScript. Relaxing these policies by adding "unsafe-eval" does fix the issue.

(it also match the behavior of the test gitea instance, since there is no CSP there)

Maybe not depending on "eval()" code would be a good thing in the future, this does fix the issue in that particular case.

[wxiaoguang]

Hmm .... some frameworks need eval, there is no eval in Gitea's JS code (web_src/js).

[techknowlogick]

@CleyFaye are you also able to post the CSP headers that you are applying?

[CleyFaye]

Yes. I trimmed it down to what seems to be the minimum extra stuff for gitea to operate:

Content-Security-Policy: default-src 'self'; img-src 'self' *.gravatar.com; media-src 'self'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; frame-ancestors 'none'; connect-src 'self'; manifest-src 'self' data:

Removing either "unsafe-inline" or "unsafe-eval" from script-src results in errors in the console.

[silverwind]

Might be worth searching the dependency code for eval use and report those to to them.

[wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf]

Yes. I trimmed it down to what seems to be the minimum extra stuff for gitea to operate:

Content-Security-Policy: default-src 'self'; img-src 'self' *.gravatar.com; media-src 'self'; style-src 'self' 'unsafe-inline'; font-src 'self' data:; script-src 'self' 'unsafe-inline' 'unsafe-eval'; frame-ancestors 'none'; connect-src 'self'; manifest-src 'self' data:

Removing either "unsafe-inline" or "unsafe-eval" from script-src results in errors in the console.

confirming that running Gitea with CSP headers not containing 'unsafe-inline' 'unsafe-eval' (still) results in errors.
as a matter of fact, I have been trying to tweak CSP for Gitea for quite some time now but to this day I, too, have been unable to get rid of the Evil Twin Tandem (unsafe-{inline,eval}) and retain a fully functional instance.

additionally, that practically means all instances (even with CSP configured as tightly as possible to still have working buttons etc.) are vulnerable to XSS in one form or another [1], which doesn't exactly cheer me up.

as was (in part) previously mentioned, the solution would be:

• a): to get rid of the code (or frameworks having code) that utilises eval(), of which I assume there to be heaps (at least as I glimpsed when ploughing through DevTools Console's error messages, trying to get to the hashes of scripts/styles that were blocked by CSP and would need to be added to allowlist)

OR

• b): to begin (perhaps optionally, for starters) setting CSP headers with pre-computed hashes or nonces for scripts/stylesheets and pages with inline scripts/styles Gitea-side, eventually enabling the user (instance admin) to tweak/add supplementary directives in the config (app.ini) as needed.

while I feel either of the proposed solutions would be greatly improving the security grounding of Gitea as a widely-deployed internet-facing service, I am aware it would require quite some time and energy to evaluate, implement and review.

perhaps it is still better than nothing to at least start a discussion.

just my two cents.

ref:
[1]: https://web.dev/csp/#eval-too

[silverwind]

unsafe-eval: Problem is really we can't really force our third-party JS dependencies to not use eval other than switching to different dependencies.
unsafe-inline: For script-src, we should be able to fix all cases of inline JS, there are certainly a few left. For style-src a exception will need to be added regardless as many mechanisms rely on inline styles, and I also see no inherent problem with those.

[wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf]

unsafe-eval: Pproblem is really we can't really force our third-party JS dependencies to not use eval other than switching to different dependencies.

not force but maybe point out the issue, etc.

unsafe-inline: For script-src, we should be able to fix all cases of inline JS, there are certainly a few left.

that is great 🚀

For style-src a exception will need to be added regardless as many mechanisms rely on inline styles, and I also see no inherent problem with those.

I get that certain mechanisms utilise this extensively (and getting rid of them would be a longer-term effort), however, the issues with those have also been made public [1], [2], [3], such as inline styles enabling a csrf_token exfil, which should IMO be considered alarming.

refs:
[1]: https://web.dev/csp/#inline-code-is-considered-harmful
[2]: https://owasp.org/www-project-web-security-testing-guide/v41/4-Web_Application_Security_Testing/11-Client_Side_Testing/05-Testing_for_CSS_Injection
[3]: https://scarybeastsecurity.blogspot.com/2009/12/generic-cross-browser-cross-domain.html

[silverwind]

not force but maybe point out the issue, etc.

Sure, maybe as a start, someone could collect all current uses of eval in gitea JS so we can raise them to the authors or think about replacements. Should be greppable in the compiled JS.

I get that certain mechanisms utilise this extensively (and getting rid of them would be a longer-term effort), however, the issues with those have also been made public [1], [2], [3], such as inline styles enabling a csrf_token exfil, which should IMO be considered alarming.

Agree, but it'll be a big refactor to even get inline-styles out of first-party code. For example jQuery show and hide rely on it which is used extensively. But that specifically, I wanted to refactor anyways.

[wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf]

Sure, maybe as a start, someone could collect all current uses of eval in gitea JS so we can raise them to the authors or think about replacements. Should be greppable in the compiled JS.

awesome, that should be possible.
to reiterate the recommendations of Eval too section, we should ideally get rid of all of eval(), new Function(), setTimeout([string], …), setInterval([string], ...).

Agree, but it'll be a big refactor to even get inline-styles out of first-party code. For example jQuery show and hide rely on it which is used extensively. But that specifically, I wanted to refactor anyways.

indeed, not to play it down, it for sure will be a big one, but that is to be expected considering not only how extensively those features were being used throughout the codebase but also the course of time (years) over which this type of code was slowly being added.
nobody says it has to go away overnight, nor that it could, nor that all of it uncoditionally should.
just that we might want to start moving in a direction that will lead us there as soon as circumstance allows.

[ghost]

I'm very sorry but this has to be fixed if gitea want's to remain "best in Class"

[wxiaoguang]

Maybe it's more complex than it looks.

From the source code of index.js, you can see there is something like try{new Function(e,"")}catch(a){i("invalid function parameter expression: ", it comes from Vue's compiler. According to MDN unsafe-eval, Function is considered as unsafe. But the Vue compiler is necessary at the moment because many Gitea pages use dynamic Vue templates, a lot of codes are mixed together. If I understand correctly, unless there is a big refactoring for frontend, otherwise it's impossible to get rid of the unsafe-eval problem.

Vue reference: https://stackoverflow.com/questions/68459611/how-to-fix-unsafe-eval-error-with-vue3-for-the-client-side-version , make Vue templates pre-compiled to drop the compiler.

And, to be honest, I do not think the CSP unsafe-eval problem is a blocker or harmful: no Gitea frontend code depends or uses it, so it doesn't cause security problem for Gitea at the moment (if there is any security problem, please report).

Conclusion: refactor to resolve it fundamentally.

[silverwind]

Precompiling Vue templates does sound like the solution and it will also increase performance because we don't need to ship and compile templates at runtime. I'm sure there must be some webpack plugin to make it happen.

Maybe like https://stackoverflow.com/a/70665152/808699, but I'm not sure we can completely eliminate the runtime compiler as I recall there are some non-SFC components and those can not be pre-compiled without rewriting them.

[wxiaoguang]

One of the challenge is DashboardRepoList, some parts are generated by Go template, and then mixed with Vue and jQuery + Fomantic UI

• https://github.com/go-gitea/gitea/blob/main/templates/user/dashboard/repolist.tmpl
• https://github.com/go-gitea/gitea/blob/main/templates/repo/branch_dropdown.tmpl

[silverwind]

Yeah, that abomination needs to be rewritten as pure SFC.

[wxiaoguang]

Update: this issue (update: only the 'unsafe-eval' part) will be resolved by:

• Refactor branch/tag selector to Vue SFC #23421
• Refactor dashboard repo list to Vue SFC #23405