Fix fuzzing issues #473
Fix fuzzing issues #473
Conversation
loosebazooka
commented
Aug 3, 2023
•
loosebazooka
commented
- handle invalid json for rekor response
- handle bundle with no tlog entry
loosebazooka
force-pushed
the
fix-a-fuzzing-issue
branch
from
35fdf83 to
781b03b
Compare
loosebazooka
changed the title
| try { | ||
| entryMap = GSON.get().fromJson(rawResponse, type); | ||
| } catch (JsonSyntaxException jse) { | ||
| throw new RekorParseException("Rekor entry json could not be parsed", jse); |
There was a problem hiding this comment.
WDYT of mentioning the problematic response in the error message?
There was a problem hiding this comment.
I think the stack trace should show it? but yeah I can just do jse.getMessage() too. I'll add that in.
loosebazooka
force-pushed
the
fix-a-fuzzing-issue
branch
from
781b03b to
454990c
Compare
|
turns out parsing bad data can create npes too. |
|
I think we can logger.error the whole rekor response? Is that what you were kinda looking for? |
loosebazooka
force-pushed
the
fix-a-fuzzing-issue
branch
3 times, most recently
from
742382d to
d1e75d2
Compare
| log.severe("Rekor entry could not be parsed"); | ||
| log.severe(rawResponse); | ||
| throw new RekorParseException("Rekor entry json could not be parsed: " + ex.getMessage(), ex); |
There was a problem hiding this comment.
Logging messages as several lines would be hard to analyze.
I suggest skip logging here.
| log.severe("Rekor entry could not be parsed"); | |
| log.severe(rawResponse); | |
| throw new RekorParseException("Rekor entry json could not be parsed: " + ex.getMessage(), ex); | |
| throw new RekorParseException("Rekor entry json could not be parsed: " + rawResponse, ex); |
ex.getMessage() would be automatically included as a part of Caused by:..., so explicitly concatenating ex.getMessage() is not useful.
There was a problem hiding this comment.
sgtm... I imagine that response could be pretty huge, but whatever, this appears to be a pretty edge error case.
- Catch parsing exceptions when handling rekor response - Check bundle before reading first tlog entry Signed-off-by: Appu Goundan <appu@google.com>
loosebazooka
force-pushed
the
fix-a-fuzzing-issue
branch
from
d1e75d2 to
53dd398
Compare
| Map<String, RekorEntry> entryMap; | ||
| try { | ||
| entryMap = GSON.get().fromJson(rawResponse, type); | ||
| } catch (JsonSyntaxException | NullPointerException | StringIndexOutOfBoundsException ex) { |
There was a problem hiding this comment.
we should report npe to gson team
|
I think fuzzing caught a number format exception issue, but I'll fix that in a followup. |
