-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
[cups] Migrate harness to upstream OpenPrinting project #12036
Conversation
integrating existing fuzz harness for CUPS into OpenPrinting/fuzzing project Signed-off-by: TTFISH <jiongchiyu@gmail.com>
fish98 is a new contributor to projects/cups. The PR must be approved by known contributors before it can be merged. The past contributors are: pkillarjun, 0x34d |
Initial contributors are for build fixes, updating harness, and adding new targets. |
Signed-off-by: TTFISH <jiongchiyu@gmail.com> fix typo in project.yaml cause error arch setting Signed-off-by: TTFISH <jiongchiyu@gmail.com> disable undefined sanitizer for now Signed-off-by: TTFISH <jiongchiyu@gmail.com>
Thanks for understanding, @pkillarjun 🙏🏻! We hope to see PRs from you in the new repository! |
auto_ccs: | ||
- "ajsinghyadav00@gmail.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be taking @pkillarjun off?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be taking @pkillarjun off?
Yes.
I think it would be better if one of the maintainers of the cups approved these changes, isn't it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be taking @pkillarjun off?
During our group meeting last week, we suppose exposing the unfixed bugs to members outside the OpenPrinting security team could potentially pose security risks and would bring maintainance inconvenience. Therefore, though we greatly appreciate Arjun's contribution, we have currently removed him from the mailing list. @tillkamppeter @iosifache
I will try. I think it would be better for me to revisit when you fellows are done. |
projects/cups/build.sh
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need a unnecessary file. You can copy $SRC/fuzzing/cups/oss_fuzz_build.sh
as $SRC/build.sh
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As many other projects e.g., cncf-fuzzing still keep this file as an entry, perhaps we can just leave this file in case of any future issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know, dude. Ethereum works fine.
projects/cups/Dockerfile
Outdated
@@ -16,7 +16,7 @@ | |||
FROM gcr.io/oss-fuzz-base/base-builder | |||
RUN apt-get update && apt-get install -y autoconf libtool-bin pkg-config zlib1g-dev libavahi-client-dev libsystemd-dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
autoconf libtool-bin pkg-config
can be removed. They are useless and waste time in the build process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed. I have removed them and tested
2. test undefined sanitizer 3. disable undefined sanitizer for now Signed-off-by: TTFISH <jiongchiyu@gmail.com>
cp FuzzIPP_seed_corpus.zip $OUT/FuzzIPP_seed_corpus.zip | ||
cp FuzzRaster_seed_corpus.zip $OUT/FuzzRaster_seed_corpus.zip | ||
popd | ||
$SRC/fuzzing/cups/oss_fuzz_build.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to get rid of this file. Or keep it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine to get rid of this file. Or keep it.
Thanks for you suggestion, I will reorganize the files in the next PR.
The OpenPrinting project under the Linux Foundation has initiated the OpenPrinting fuzzing repository to officially maintain OSS-Fuzz testing tasks. This PR includes following updates:
All harness building related changes has been locally tested and verified.
We deeply appreciate the contributions from @pkillarjun for his initial integration of the CUPS project into OSS-Fuzz and his support during this migration. However, OpenPrinting has decided to move the corpus, fuzzing harnesses, and build scripts into the repository under its organisation and maintainance. In addition, the bug reports will be firstly processed by OpenPrinting's security team and not by the initial contributor, who is not part of the organisation. These two measures will minimise the security risks and still maintaining the ability of open source fellows to participate in the development.
CC @tillkamppeter @iosifache