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

fixed avatar eicar check #5779

Merged
merged 2 commits into from
Apr 15, 2024
Merged

Conversation

SilentFlameCR
Copy link
Contributor

setAvatar now sending original_avatar and eidted avatar for the clamav check. Scanning for viruses fixed for avatar.

Copy link

sonarcloud bot commented Apr 15, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@farhatahmad farhatahmad merged commit 6ddd441 into bigbluebutton:master Apr 15, 2024
3 checks passed
@defnull
Copy link
Contributor

defnull commented Apr 15, 2024

The avatar that is actually stored and later displayed to visitors is not scanned? I can simply send a normal image as user[original_avatar] and a malicious one as user[avatar] to bypass the clamav check.

@farhatahmad
Copy link
Collaborator

farhatahmad commented Apr 15, 2024

It's still scanned in the user.rb file as a backup

def scan_avatar_for_virus
return if !virus_scan? || !attachment_changes['avatar']
path = attachment_changes['avatar']&.attachable&.tempfile&.path
return true if Clamby.safe?(path)
errors.add(:avatar, 'MalwareDetected')
throw :abort

@defnull
Copy link
Contributor

defnull commented Apr 16, 2024

Ah, so this is just for generating a proper error message on upload, I see. Thanks for the explanation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants