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

Clam 938 velvet sweatshop #700

Merged
merged 1 commit into from
Oct 21, 2022
Merged

Conversation

ragusaa
Copy link
Contributor

@ragusaa ragusaa commented Sep 20, 2022

Draft PR. NOT ready for review.

@val-ms val-ms marked this pull request as draft September 22, 2022 18:03
@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch 2 times, most recently from ae4e1b5 to db73947 Compare September 27, 2022 20:25
@lgtm-com
Copy link

lgtm-com bot commented Sep 27, 2022

This pull request introduces 1 alert when merging db73947 into 197113c - view on LGTM.com

new alerts:

  • 1 for Multiplication result converted to larger type

@val-ms val-ms marked this pull request as ready for review September 29, 2022 20:57
libclamav/ole2_extract.c Show resolved Hide resolved
libclamav/ole2_extract.c Show resolved Hide resolved
libclamav/ole2_extract.c Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/scanners.c Outdated Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 7, 2022

This pull request introduces 1 alert when merging 3d9f3e6 into b3a3b35 - view on LGTM.com

new alerts:

  • 1 for Multiplication result converted to larger type

@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch 3 times, most recently from 7ec03e0 to ac87635 Compare October 10, 2022 18:20
@lgtm-com
Copy link

lgtm-com bot commented Oct 10, 2022

This pull request fixes 1 alert when merging ac87635 into b3a3b35 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch from bbdf270 to 8ab4012 Compare October 10, 2022 19:29
Copy link
Contributor

@val-ms val-ms left a comment

Choose a reason for hiding this comment

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

Outside of some very minor issues, this looks awesome. Very nice work, @ragusaa

I'm happy with the code review.

Next up,

  1. I would like if you could can add a non-malware test case for the supported encryption type(s). If you can provide the samples/signatures, I'd be happy to create the test itself under clamscan_test.,py, or else you are welcome to do it.
  2. We should do some regression testing on a big set from the zoo, and see what happens.

libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
libclamav/ole2_extract.c Outdated Show resolved Hide resolved
@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch from 4741cb1 to 7044d86 Compare October 13, 2022 15:03
@lgtm-com
Copy link

lgtm-com bot commented Oct 13, 2022

This pull request fixes 1 alert when merging 7044d86 into a4e6868 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch 2 times, most recently from c491b70 to 2694f89 Compare October 14, 2022 15:07
@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

This pull request fixes 1 alert when merging 2694f89 into cf81299 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

This pull request fixes 1 alert when merging 3d91a04 into cf81299 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch from 6baa73f to 13d1290 Compare October 14, 2022 20:15
@lgtm-com
Copy link

lgtm-com bot commented Oct 14, 2022

This pull request fixes 1 alert when merging 13d1290 into cf81299 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@Cisco-Talos Cisco-Talos deleted a comment from lgtm-com bot Oct 14, 2022
@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch 3 times, most recently from 13b473f to 47b9b0b Compare October 21, 2022 16:26
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request fixes 1 alert when merging 47b9b0b into 449bcd2 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request fixes 1 alert when merging 25eef13 into 449bcd2 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@ragusaa ragusaa force-pushed the CLAM-938-VelvetSweatshop branch from 25eef13 to 276ee61 Compare October 21, 2022 19:21
@lgtm-com
Copy link

lgtm-com bot commented Oct 21, 2022

This pull request fixes 1 alert when merging 276ee61 into 449bcd2 - view on LGTM.com

fixed alerts:

  • 1 for Multiplication result converted to larger type

@val-ms
Copy link
Contributor

val-ms commented Oct 21, 2022

For any readers -- we moved the test to our internal repo, because the (entirely non-malicious) test file Andy created is detected by some antivirus for being understandably suspicious. We don't want the clam source, or even decrypted clam test files to be flagged by AV's.

Testing looked good in the jenkins pipelines. Merging.

@val-ms val-ms merged commit e16a552 into Cisco-Talos:main Oct 21, 2022
@SecT0uch
Copy link
Contributor

SecT0uch commented Oct 13, 2023

I'm having a sample that seems to be encrypted with VelvetSweatshop but clamav outputs "EncryptedWithVelvetSweatshop":0.

Is this because of LibClamAV debug: ole2: Invalid second bit, must be 0 ?

msoffcrypto-tool file.xls out.xls -p VelvetSweatshop works with no issue.

You can download the sample here: https://bazaar.abuse.ch/sample/ef30b686955d11c92ab89e6c5c5e4e61fc3d9797aca3d16d3011a1a6474847a6/

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.

3 participants