Skip to content

bug: fix stored XSS in About page feedback slideshow #331

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

Open
devin-ai-integration wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

bug: fix stored XSS in About page feedback slideshow #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 6 additions & 1 deletion frontend/src/app/about/about.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ <h2 id="customer-feedback"><span>{{"SECTION_CUSTOMER_FEEDBACK" | translate}}</sp
>
<ng-container *galleryImageDef="let item; let active = active">
@if (active) {
<figure class="feedback" [innerHTML]="item?.args"></figure>
<figure class="feedback">
<figcaption>
<p class="feedback-comment">{{ item?.args?.comment }}</p>
<div class="feedback-stars">(@for (star of item?.args?.stars ?? []; track $index) {<i class="{{ star }}" aria-hidden="true"></i>})</div>
</figcaption>
</figure>
}
</ng-container>
</gallery>
Expand Down
22 changes: 22 additions & 0 deletions frontend/src/app/about/about.component.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { MatCardModule } from '@angular/material/card'

import { of } from 'rxjs'
import { ConfigurationService } from '../Services/configuration.service'
import { FeedbackService } from '../Services/feedback.service'
import { provideHttpClient, withInterceptorsFromDi } from '@angular/common/http'

import { AboutComponent } from './about.component'
Expand All @@ -20,9 +21,14 @@ describe('AboutComponent', () => {
let component: AboutComponent
let fixture: ComponentFixture<AboutComponent>
let configurationService
let feedbackService
let translateService

beforeEach(async () => {
feedbackService = {
find: vi.fn().mockName("FeedbackService.find")
}
feedbackService.find.mockReturnValue(of([]))
configurationService = {
getApplicationConfiguration: vi.fn().mockName("ConfigurationService.getApplicationConfiguration")
}
Expand All @@ -43,6 +49,7 @@ describe('AboutComponent', () => {
TranslateModule.forRoot()],
providers: [
{ provide: ConfigurationService, useValue: configurationService },
{ provide: FeedbackService, useValue: feedbackService },
{ provide: TranslateService, useValue: translateService },
provideHttpClient(withInterceptorsFromDi()),
provideHttpClientTesting()
Expand Down Expand Up @@ -116,4 +123,19 @@ describe('AboutComponent', () => {

expect(component.nftUrl).toBe('NFT')
})

it('should pass feedback comments to the gallery as plain text without marking them as trusted HTML', () => {
feedbackService.find.mockReturnValue(of([{ comment: '<iframe src="javascript:alert(`xss`)">', rating: 2 }]))
component.galleryRef = { addImage: vi.fn().mockName('GalleryRef.addImage') } as any

component.populateSlideshowFromFeedbacks()

expect(component.galleryRef.addImage).toHaveBeenCalledWith({
src: 'assets/public/images/carousel/1.jpg',
args: {
comment: '<iframe src="javascript:alert(`xss`)">',
stars: ['fas fa-star', 'fas fa-star', 'far fa-star', 'far fa-star', 'far fa-star']
}
})
Comment on lines +127 to +139

@devin-ai-integration devin-ai-integration Bot Sep 10, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 Regression test skips rendering sink

The test checks addImage arguments but never renders the feedback template. Restoring [innerHTML] would leave this assertion passing.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

})
})
32 changes: 13 additions & 19 deletions frontend/src/app/about/about.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
*/

import { Component, type OnInit, inject } from '@angular/core'
import { DomSanitizer } from '@angular/platform-browser'
import { ConfigurationService } from '../Services/configuration.service'
import { FeedbackService } from '../Services/feedback.service'
import { Gallery, type GalleryRef, GalleryComponent, GalleryImageDef } from 'ng-gallery'
Expand All @@ -30,7 +29,6 @@ library.add(faFacebook, faTwitter, faSlack, faReddit, faNewspaper, faStar, fasSt
export class AboutComponent implements OnInit {
private readonly configurationService = inject(ConfigurationService)
private readonly feedbackService = inject(FeedbackService)
private readonly sanitizer = inject(DomSanitizer)
private readonly gallery = inject(Gallery)

public blueSkyUrl?: string
Expand All @@ -53,14 +51,7 @@ export class AboutComponent implements OnInit {
'assets/public/images/carousel/7.jpg'
]

private readonly stars = [
null,
'<i class="fas fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i>',
'<i class="fas fa-star"></i><i class="fas fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i>',
'<i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i><i class="far fa-star"></i><i class="far fa-star"></i>',
'<i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i><i class="far fa-star"></i>',
'<i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i><i class="fas fa-star"></i>'
]
private readonly maxRating = 5

ngOnInit (): void {
this.galleryRef = this.gallery.ref('feedback-gallery')
Expand Down Expand Up @@ -112,19 +103,22 @@ export class AboutComponent implements OnInit {
)
.subscribe((feedbacks) => {
for (let i = 0; i < feedbacks.length; i++) {

feedbacks[i].comment = `<figcaption><p class="feedback-comment">${
feedbacks[i].comment
}</p><div class="feedback-stars">(${this.stars[feedbacks[i].rating]})</div></figcaption>`
feedbacks[i].comment = this.sanitizer.bypassSecurityTrustHtml(
feedbacks[i].comment
)

this.galleryRef.addImage({
src: this.images[i % this.images.length],
args: feedbacks[i].comment
args: {
comment: feedbacks[i].comment,
stars: this.starIcons(feedbacks[i].rating)
}
})
}
})
}

private starIcons (rating: number): string[] {
const icons: string[] = []
for (let i = 1; i <= this.maxRating; i++) {
icons.push(i <= rating ? 'fas fa-star' : 'far fa-star')
}
return icons
}
}
Loading