-
Notifications
You must be signed in to change notification settings - Fork 164
feat: add hook for Enum.valueOf #1031
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -866,6 +866,53 @@ public static void mapGetOrDefault( | |||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| @MethodHook( | ||||||||||||||||||||||||
| type = HookType.BEFORE, | ||||||||||||||||||||||||
| targetClassName = "java.lang.Enum", | ||||||||||||||||||||||||
| targetMethod = "valueOf", | ||||||||||||||||||||||||
| targetMethodDescriptor = "(Ljava/lang/Class;Ljava/lang/String;)Ljava/lang/Enum;") | ||||||||||||||||||||||||
| public static void enumValueOf( | ||||||||||||||||||||||||
| MethodHandle method, Object alwaysNull, Object[] arguments, int hookId) { | ||||||||||||||||||||||||
| if (arguments.length < 2) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Object enumTypeArg = arguments[0]; | ||||||||||||||||||||||||
| Object nameArg = arguments[1]; | ||||||||||||||||||||||||
| if (!(enumTypeArg instanceof Class) || !(nameArg instanceof String)) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| Class<?> enumClass = (Class<?>) enumTypeArg; | ||||||||||||||||||||||||
| if (enumClass == null || !enumClass.isEnum()) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| String candidate = (String) nameArg; | ||||||||||||||||||||||||
| if (candidate == null) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
Comment on lines
+885
to
+891
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These
Suggested change
Respectively, given that the hook specifies |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Enum<?>[] constants; | ||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||
| constants = (Enum<?>[]) enumClass.getEnumConstants(); | ||||||||||||||||||||||||
| } catch (Exception | LinkageError ignored) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| if (constants == null || constants.length == 0) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Skip guidance if the target string is already valid. | ||||||||||||||||||||||||
| for (Enum<?> enumConstant : constants) { | ||||||||||||||||||||||||
| if (enumConstant.name().equals(candidate)) { | ||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| // Guide the fuzzer towards a single valid enum | ||||||||||||||||||||||||
| int index = Math.floorMod(candidate.hashCode(), constants.length); | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can jump around when candidate changes slightly, which could throw off the guidance. Instead, you could use a ClassValue to associate an Enum class with a sorted array of its values and then
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am curious about the reasoning to go for the nearest enum value--why prefer it over any other enum value? I am asking because I think there is a bias in libfuzzer's output (e.g. zeros are way more favored than anything else), and by taking the closest value, this bias gets propagated. WDYT?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Picking the nearest candidate isn't necessary, but I think that a particular
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I benchmarked the 3 solutions (binary search, random pick, hashcode based pick) by executing 50 times a fresh fuzz test with 1M executions. Binary search with random pick for the neighbour: Hashcode pick (random pick has also very similar values): |
||||||||||||||||||||||||
| Enum<?> target = constants[index]; | ||||||||||||||||||||||||
| TraceDataFlowNativeCallbacks.traceStrcmp(candidate, target.name(), 1, hookId); | ||||||||||||||||||||||||
| } | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| private static final class Bounds { | ||||||||||||||||||||||||
| private final Object lower; | ||||||||||||||||||||||||
| private final Object upper; | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,39 @@ | ||||||
| /* | ||||||
| * Copyright 2024 Code Intelligence GmbH | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be 2026? (or does it not matter much?)
Suggested change
|
||||||
| * | ||||||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||||||
| * you may not use this file except in compliance with the License. | ||||||
| * You may obtain a copy of the License at | ||||||
| * | ||||||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||||||
| * | ||||||
| * Unless required by applicable law or agreed to in writing, software | ||||||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||||||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||||
| * See the License for the specific language governing permissions and | ||||||
| * limitations under the License. | ||||||
| */ | ||||||
|
|
||||||
| package com.example; | ||||||
|
|
||||||
| import com.code_intelligence.jazzer.api.FuzzerSecurityIssueMedium; | ||||||
| import com.code_intelligence.jazzer.mutation.annotation.NotNull; | ||||||
|
|
||||||
| public class EnumFuzzer { | ||||||
| private enum SampleColor { | ||||||
| RED, | ||||||
| GREEN, | ||||||
| BLUE, | ||||||
| SUPER_SECRET_SHADE | ||||||
| } | ||||||
|
|
||||||
| public static void fuzzerTestOneInput(@NotNull String value) { | ||||||
| try { | ||||||
| SampleColor color = SampleColor.valueOf(value); | ||||||
| if (color == SampleColor.SUPER_SECRET_SHADE) { | ||||||
| throw new FuzzerSecurityIssueMedium("Enum matched: " + color); | ||||||
| } | ||||||
| } catch (IllegalArgumentException ignored) { | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
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.
Out of curiositry: Given that this hook specifies
targetMethodDescriptor, is it even possible thatarguments.length < 2is ever true?