ER: resolve CodeQL alerts 36 and 37 based on analysis from #5297

[roslynwythe]

Emergent Requirement - Problem

CodeQL raised alerts regarding the script babel.js which is loaded from the cloudflare CDN. A code fix was recommended but before implementing we need to ensure that the babel.js script is still required and should still be served from a CDN. . Even if Babel is required, there may be a more efficient means of applying it, prior to the browser.

Details

Babel is a toolchain that is mainly used to convert ECMAScript 2015+ code into a backwards compatible version of JavaScript in current and older browsers or environments. With most modern browsers have already implemented nearly all features of es6, the requirement for Babel is reduced greatly, but Babel does perform other functions including transforming syntax, Code mods and polyfills.

CodeQL alerts:
https://github.com/hackforla/website/security/code-scanning/36
https://github.com/hackforla/website/security/code-scanning/37

The alerts were raised in _layouts/default-markdown.html and _layouts/default.html. It is not clear if _layouts/default-markdown.html is in current use.

Issue you discovered this emergent requirement in

• Analysis for CodeQL query Inclusion of functionality from an untrusted source #5297

Date discovered

Did you have to do something temporarily

• YES
• NO

Who was involved

@freaky4wrld @roslynwythe

What happens if this is not addressed

The security concern is that if an attacker were to gain control of the CDN, a corrupted script could be loaded into any Hack for LA webpage

Resources

• Analysis for CodeQL query Inclusion of functionality from an untrusted source #5297
• https://github.com/hackforla/website/security/code-scanning/36
• https://github.com/hackforla/website/security/code-scanning/37
• https://babeljs.io/docs
• https://medium.com/@prevaj_29198/do-we-need-babel-in-every-project-19b083ab4f06
• https://medium.com/@codingstorytime/set-up-a-build-environment-using-babel-and-webpack-172110681b1

Recommended Action Items

• Determine if Babel.js is still required
  • If babel is still required, does it have to be served from a CDN? Could it be applied at build time?
  • Is the file _layouts/default-markdown.html still in use? If not, create an issue to remove it, and dismiss alert 36
• If we still need to load babel.js from a CDN, make a new issue(s) for updating the code as required to resolve each alert and perform testing to ensure that the solution is effective
• Let a Team Lead know

Potential solutions [draft]

Code change

add the attributes:

integrity="generated-sri-hash" crossorigin="anonymous"

to the relevant script tags

Testing:

• the most simple would be using the console and look for any issues with the script or its integrity
• another one could be using the `network tab and look for the external scripts failure
• maybe using a fallback mechanism with <noscript> tag indicating a failure in the script loading
• Or the best would be deliberately tampering the babel script or introduce an error in the script URL to simulate a tampered or failed script and observing its behaviour.

[github-actions]

Hi @freaky4wrld, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:-
i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?)
ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

[freaky4wrld]

Availability: Evenings
ETA: 11/25/2023

[freaky4wrld]

Hey there @roslynwythe , just want to discuss some points with you about my observations : -

• According to my observation for the working of babel.js the line mentioned in the CodeQL scans 36 and 37, omitting the line<script src="https://cdnjs.cloudflare.com/ajax/libs/babel-standalone/6.26.0/babel.js"></script> ' doesn't create any issue in the build process and no dependencies are found dependent on babel.js. But no error message is found in the network tab`. Omitting the mentioned line.
• However the file _layouts/default-markdown.html is used as a layout in the file pages/code-of-conduct.html and effects the Code of Conduct page, thus in my analysis the file is needed.

The only thing left is your further discussion with Bonnie. I hope the observations are helpful enough for you. DM me for further discussions, always glad to help you.

[roslynwythe]

@freaky4wrld Since babel.js is a compiler/transpiler for JavaScript, an alternative to invoking it in the browser would be to run it in the build process as described in https://medium.com/@codingstorytime/set-up-a-build-environment-using-babel-and-webpack-172110681b1. So I'm wondering if we should write an issue to investigate the pros and cons of that approach.

If we do write an issue to resolve the alerts using the "integrity" attribute, do we need to specify a value for the hash?

[freaky4wrld]

@roslynwythe I think writing an issue to investigate the pros and cons would be better.
And yes, we have to provide hash to the integrity attribute in the script tags.

[roslynwythe]

@freaky4wrld please proceed to write a new issue to investigate the pros and cons of moving babel to the build. While you are writing the issue, apply the Draft label to it, and self-assign it. When it is ready for approval, remove the Draft label and let me know via Slack message or apply the ready for dev lead label. At that point you would typically remove yourself as the assignee, but if you wish to remain assigned and actually work on the issue, mention that in a comment.

Once the new issue is created, please create a comment in this ER, referencing the issue number. Typically, once an issue is created, the associated ER would be closed, but in this case, this ER will remain open until the "investigation" issue is complete and we decide how to proceed. We will move the ER to the "Ice Box" during that time and we unassign you from it, but if you wish to take it up again once the "investigation" issue is complete, indicate that in a comment and we will give you first chance to assign it.

[freaky4wrld]

• @roslynwythe the new investigatory issue has been created please check it out Investigate the Pros and Cons of moving babel.js to the build process #5979

[fyliu]

@freaky4wrld I thought this comment was saying that babel was not needed by the website project and that the default-markdown layout file was still needed.

But maybe a discussion on slack or zoom decided babel is still needed.

What about just committing the babel.js file into the website repo and point to that file? It'd be another dependency to be updated on some schedule.

I'm just pointing out an alternative to:

• pointing to babel.js hosted by a 3rd party
• moving babel into the build process

[roslynwythe]

Thank you @fyliu for raising a viable and much simpler alternative to moving the babel into the build process. Moving babel into the build process would have the advantage that the dependabot would notify regarding version updates, but that solution would require a good deal of effort and complexity that doesn't seem worth it, given that the CodeQL alert have been resolved using the sri attribute.

• @freaky4wrld @ExperimentsInHonesty since the CodeQL alerts have been resolved via Implemented SRI check on the external babel links #6152 I suggest closing this ER.

[freaky4wrld]

@roslynwythe yes we should close the ER