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

Website: Clean Air Forum 2025 #2437

Merged
merged 1 commit into from
Feb 4, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 61 additions & 53 deletions website2/src/views/Forum/AboutPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,82 +5,90 @@ import { Divider, NoData } from '@/components/ui';
import { useForumData } from '@/context/ForumDataContext';
import { renderContent } from '@/utils/quillUtils';

// Reusable component for a two-column row with a bold title on the left.
type SectionRowProps = {
title: string;
children: React.ReactNode;
};

const SectionRow: React.FC<SectionRowProps> = ({ title, children }) => (
<div className="py-4 flex flex-col md:flex-row items-start transition duration-150 ease-in-out hover:bg-gray-50 rounded">
<div className="md:w-1/3 mb-2 md:mb-0 text-left text-xl font-bold">
{title}
</div>
<div className="md:w-2/3 text-left space-y-4">{children}</div>
</div>
);

const AboutPage = () => {
const data = useForumData();

if (!data || !data.introduction) {
return <NoData />;
}

// Render the objectives list
const renderObjectives = () => (
<div className="space-y-4">
<h2 className="text-2xl font-bold text-gray-900">
{data?.engagements?.title || 'Objectives'}
</h2>
<div>
{data?.engagements?.objectives?.map((objective: any, index: number) => (
<p key={index}>{objective.details || ''}</p>
))}
</div>
</div>
);
console.info(data);

// Objectives Section: Render each objective as a SectionRow
const renderObjectives = () => {
const objectives = data?.engagement?.objectives || [];
return (
<section className="space-y-6">
<h2 className="text-2xl font-bold text-left">Objectives</h2>
<div className="divide-y divide-gray-200">
{objectives.map((objective: any) => (
<SectionRow key={objective.id} title={objective.title}>
{objective.details}
</SectionRow>
))}
</div>
</section>
);
};
Comment on lines +33 to +47
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Type safety improvements needed in objectives mapping

The use of any type in the objectives mapping could lead to runtime errors. Let's properly type this for better type safety.

+ type Objective = {
+   id: string | number;
+   title: string;
+   details: string;
+ };

  const renderObjectives = () => {
-   const objectives = data?.engagement?.objectives || [];
+   const objectives: Objective[] = data?.engagement?.objectives || [];
    return (
      <section className="space-y-6">
        <h2 className="text-2xl font-bold text-left">Objectives</h2>
        <div className="divide-y divide-gray-200">
-         {objectives.map((objective: any) => (
+         {objectives.map((objective: Objective) => (
            <SectionRow key={objective.id} title={objective.title}>
              {objective.details}
            </SectionRow>
          ))}
        </div>
      </section>
    );
  };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const renderObjectives = () => {
const objectives = data?.engagement?.objectives || [];
return (
<section className="space-y-6">
<h2 className="text-2xl font-bold text-left">Objectives</h2>
<div className="divide-y divide-gray-200">
{objectives.map((objective: any) => (
<SectionRow key={objective.id} title={objective.title}>
{objective.details}
</SectionRow>
))}
</div>
</section>
);
};
type Objective = {
id: string | number;
title: string;
details: string;
};
const renderObjectives = () => {
const objectives: Objective[] = data?.engagement?.objectives || [];
return (
<section className="space-y-6">
<h2 className="text-2xl font-bold text-left">Objectives</h2>
<div className="divide-y divide-gray-200">
{objectives.map((objective: Objective) => (
<SectionRow key={objective.id} title={objective.title}>
{objective.details}
</SectionRow>
))}
</div>
</section>
);
};


return (
<div className="w-full px-6 lg:px-0 bg-white">
<Divider className="bg-black p-0 m-0 h-[1px] w-full max-w-5xl mx-auto" />

{/* Main Content */}
<div className="max-w-5xl mx-auto space-y-12 py-8">
{/* Introduction */}
<div
dangerouslySetInnerHTML={{
__html: renderContent(data.introduction),
}}
/>
{/* Introduction Section (kept out of the redesign) */}
<section className="space-y-6">
<h2 className="text-2xl font-bold text-left">Introduction</h2>
<div
className="prose"
dangerouslySetInnerHTML={{
__html: renderContent(data.introduction),
}}
Comment on lines +60 to +62
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Security Concern: Insufficient Sanitization in renderContent

Based on our investigation:

  • The renderContent function in website2 appears in two locations:
    • ./website2/src/components/ui/MemberCard.tsx
    • ./website2/src/utils/quillUtils.ts
  • Our search shows no evidence of a sanitization library (e.g., DOMPurify) being used within these functions. The only instance of DOMPurify in the repository is in the frontend code (i.e., ./website/frontend/src/pages/CleanAir/ForumEventsPages/Resources.js), not within the website2 implementation.
  • Without proper sanitization, any HTML processed by renderContent and rendered via dangerouslySetInnerHTML is at risk of XSS vulnerabilities.

Action Items:

  • Audit renderContent Implementation: Revisit the code in ./website2/src/utils/quillUtils.ts (and possibly MemberCard.tsx) to verify if any sanitization is performed.
  • Apply Proper Sanitization: If not already done, use a well-tested library like DOMPurify to sanitize HTML before rendering.
  • Review Other Usages: Ensure that all other instances of dangerouslySetInnerHTML (e.g., in AboutPage at lines 60-62, and similar sections at lines 74-78 and 87-89) are also receiving sanitized content.

Given the lack of evidence for sanitization in website2's implementation, this poses a potential security risk.

🔗 Analysis chain

Security: Review dangerouslySetInnerHTML usage

The use of dangerouslySetInnerHTML in multiple locations could expose users to XSS attacks if the content isn't properly sanitized.

Please verify that the renderContent utility function properly sanitizes HTML content. Consider using a well-tested HTML sanitization library like DOMPurify.

Also applies to: 74-78, 87-89


I'll start by gathering more information about how the renderContent utility function is implemented and whether it makes use of a sanitization library like DOMPurify. In particular, we need to confirm that any HTML handed to dangerouslySetInnerHTML is properly cleaned before rendering.

Below is a shell script to search the codebase for the definition of renderContent and to check for any references to DOMPurify:

Once we review the output of this script, we'll be able to confirm whether the HTML passed to dangerouslySetInnerHTML is being sanitized properly.

🏁 Scripts executed

The following scripts were executed for the analysis:

Web query:

What are the best practices for safely rendering HTML content in React?

Length of output: 6741


Script:

#!/bin/bash
echo "Searching for 'renderContent' definition in the codebase..."
rg -n "function renderContent" .
rg -n "const renderContent" .

echo ""
echo "Searching for DOMPurify usage in the codebase..."
rg -n "DOMPurify" .

Length of output: 829

🧰 Tools
🪛 Biome (1.9.4)

[error] 60-60: Avoid passing content using the dangerouslySetInnerHTML prop.

Setting content using code can expose users to cross-site scripting (XSS) attacks

(lint/security/noDangerouslySetInnerHtml)

/>
</section>

{/* Objectives Section */}
{renderObjectives()}

<Divider className="bg-black p-0 m-0 h-[1px] w-full max-w-5xl mx-auto" />

{/* Split Section - Sponsorship Opportunities */}
<div>
<div className="flex flex-col md:flex-row md:space-x-8">
<div className="md:w-1/3 mb-4 md:mb-0">
<h2 className="text-2xl font-bold text-gray-900">
Sponsorship Opportunities
</h2>
</div>
<div
className="md:w-2/3"
dangerouslySetInnerHTML={{
__html: renderContent(
data?.sponsorship_opportunities_about || '',
),
}}
/>
</div>
</div>
{/* Sponsorship Opportunities Section */}
<SectionRow title="Sponsorship Opportunities">
<div
dangerouslySetInnerHTML={{
__html: renderContent(
data?.sponsorship_opportunities_about || '',
),
}}
/>
</SectionRow>

<Divider className="bg-black p-0 m-0 h-[1px] w-full max-w-5xl mx-auto" />

{/* Split Section - Sponsorship Packages */}
<div>
<div className="flex flex-col md:flex-row md:space-x-8">
<div className="md:w-1/3 mb-4 md:mb-0">
<h2 className="text-2xl font-bold text-gray-900">
Sponsorship Packages
</h2>
</div>
<div
className="md:w-2/3 space-y-4"
dangerouslySetInnerHTML={{
__html: renderContent(data?.sponsorship_packages || ''),
}}
/>
</div>
</div>
{/* Sponsorship Packages Section */}
<SectionRow title="Sponsorship Packages">
<div
dangerouslySetInnerHTML={{
__html: renderContent(data?.sponsorship_packages || ''),
}}
/>
</SectionRow>
</div>
</div>
);
Expand Down
Loading