Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
  • 18:52
    HeroicKatora commented #1577
  • 14:36
    hgaiser commented #1577
  • 14:35
    hgaiser synchronize #1577
  • 11:25
    hgaiser commented #1577
  • 11:25
    hgaiser synchronize #1577
  • 10:05
    hgaiser commented #1570
  • 09:27
    hgaiser opened #1577
  • Oct 14 20:31
    HeroicKatora commented #1575
  • Oct 14 19:39
    fintelia commented #1575
  • Oct 14 19:30
    AndreKR commented #1575
  • Oct 14 19:29
    AndreKR commented #1575
  • Oct 14 19:25
    HeroicKatora commented #1575
  • Oct 14 19:14
    fintelia commented #1575
  • Oct 14 18:34
    AndreKR commented #1575
  • Oct 14 17:54
    HeroicKatora edited #1433
  • Oct 14 17:53
    HeroicKatora commented #1575
  • Oct 14 17:39
    AndreKR commented #1575
  • Oct 14 14:38
    AndreKR commented #1575
  • Oct 14 07:34
    alexanderkjall opened #1576
  • Oct 14 07:31
    HeroicKatora commented #1575
Chris
@caemor
Hi! Is there an easy way to make an png image square (add transparent background where needed) and the resize it to a smaller size? (e.g. an 2556x1234 image should be made to a square 2556x2556 and then scaled down to 400x400)
I only found resize(_*) which does the second step, but I would also need some centered canvas size change function.
1 reply
poly000
@poly000
@caemor create a new ImageBuffer in size 2556x2556 and overlay previous one on?
Chris
@caemor
Thanks got it working with the overlay.
5225225
@r522:matrix.org
[m]

hey sorry for basically abandoning the fuzz fixes pull request for... far too long

anyways, i implemented the IFD cycle checking

as implemented, read_header and read_ifd have a new error state where if we ever see an IFD offset twice, we return it.

I've not set any form of limit on the size of the hashmap, so it can grow unbounded, but every entry we add must actually exist in the file (as in, a file can't add thousands of entries to the hashmap while only being hundreds of bytes long, unlike OOMs where we allocate a buffer of a specific size given by the file), so this might be fine?

cc HeroicKatora

one open question is if we should be using loop or for _ in 0..128 in the fuzzer

i think you said earlier to make it limited, but I'd argue that clients probably will be reading off images as an iterator without any form of limit, so if we constantly return valid images, they'll get stuck in loops

or in other words, an infinite loop of valid images is our bug

1 reply
the fuzzer seems to not run into infinite loops even with loop, and at least with the default settings of 4KB max input size, seems to not fail at all anymore
5225225
@r522:matrix.org
[m]
also, CI tests on 1.34.2, which fails on a lot of our dependencies, so that'll probably need to be bumped
1 reply
a lot being 2 that i've seen (itertools and flate2)
image-rs/image-tiff#141 is the PR, for reference
HeroicKatora
@heroic_katora:matrix.org
[m]
The largest downside I can see from that is that, of course, there might be errors beyond the number, whichever we choose.
5225225
@r522:matrix.org
[m]

IMO the limit there that they have should only be for limiting the amount of real images they care about

we should have a finite number of images in a valid file, if there's an infinite amount the file is invalid (and we report an error in that case)

or in other words, if we take input.len() limited number of images, and return once there's no more images / there's an error, we should be free to insert a panic at the end of that loop, since it's impossible (to my knowledge) to validly encode more than 1 image per byte
i'm fine with changing the fuzzer to limit the count again, I just feel that we shouldn't be saying that infinite loops when taking images is not a bug in image-tiff
1 reply
5225225
@r522:matrix.org
[m]

that works

doesn't protect against internal infinite loops (and ultimately, an infinite loop and a panic is the same, just infinite loops take longer to trigger the timeout)

which defaults to 1200 seconds but I generally set to a few seconds
1 reply

uhhh I don't think libfuzzer has a config file

I could make a shell script that calls cargo +nightly fuzz decode_image -- -timeout=5 or whatever

HeroicKatora
@heroic_katora:matrix.org
[m]
Say we treat all decoding throughput of <1kB/s as a bug. Ah, hm, then it'd need to be dynamic anyways for that goal.
Okay, anyways, shell script sounds good. I'd mention it in the readme as well.
5225225
@r522:matrix.org
[m]
we could check the current time, decode the image, then check the time again?
might add some overhead to each fuzzing run though
(and panic if the time is "too long")
i think that's overly complex though
and generally the input sizes are pretty consistent
they're limited to 4096 bytes by default anyways
unless you have a larger file in your corpus directory, in which case the size of the largest file is taken to be the limit
HeroicKatora
@heroic_katora:matrix.org
[m]
Can we query that? Well, or since the limit is manually configured specify it in the readme.
Basically we know 4kB should be done in < 4s if we enforce 1kB throughput.
Or make a PR to cargo fuzz for that functionality :) (super optional)
5225225
@r522:matrix.org
[m]
i think it would be a libfuzzer thing, since that's the thing that's actually guesssing the length
1 reply
and libfuzzer is LLVM
cargo fuzz, as i understand it, is basically just scripts to make working with libfuzzer nicer
HeroicKatora
@heroic_katora:matrix.org
[m]
Yup, that's how cargo fuzz works. So there's a reason not to build another separate layer of scripts around it ^^
5225225
@r522:matrix.org
[m]
so what would be the proposal there, guessing the length to do what?
HeroicKatora
@heroic_katora:matrix.org
[m]
So, add a flag --minimum-throughput=4kB/s to dynamically adjust the timeout based on its guess of maximum length.
I mean, in the truly best case it would adjust the limit based on the actual test case but this would probably require quite a bit more integration with libfuzzer.
5225225
@r522:matrix.org
[m]

also, i'm not entirely sure how useful the shell script that just sets the timeout is
since often when i'm running fuzzers i want to change the number of processes it runs / sanitisers

I could add a copy-pastable command line in the readme in the root (or a new README.md in the fuzz/ directory), but I'm not entirely sure how useful the shell script would be directly

ahh, yeah
HeroicKatora
@heroic_katora:matrix.org
[m]
With the motivation that throughput is something that can be a requirement (think: realtime).
Of course if you specify an explicit timeout it should override it. (Or take the minimum?)
5225225
@r522:matrix.org
[m]
libfuzzer would probably need sub-second timeouts (and to probably use CPU time, not real time) for that to be more useful
idk
HeroicKatora
@heroic_katora:matrix.org
[m]
Oh true, would really appreciate it if cargo-fuzz spawned libfuzzer in a cgroup that's in full control over a set of cores on Linux-RT :P
Yeah ^^ I'm daydreaming about cargo-fuzz so a copy-paste command line should be good enough for now
5225225
@r522:matrix.org
[m]
or run through valgrind and count CPU cycles and estimate a runtime from that
1 reply
make your fuzzing runs 10 times slower with this one easy trick
HeroicKatora
@heroic_katora:matrix.org
[m]
And a Fuzz-OS that could do this more precisely without the overhead
In any case, regarding CI failure, feel free to bump the minimum rustc version to 1.48
5225225
@r522:matrix.org
[m]
okay cool
CI now passes and i added a short note in the readme about fuzzing
HeroicKatora
@heroic_katora:matrix.org
[m]
Nice, thank you so much for that :)
5225225
@r522:matrix.org
[m]
and hopefully the turnaround for my PRs can only get faster from here :D
one thing that might be worth doing is getting a sample of known valid images and testing that we do parse them fine
because all fuzzing can really do is test that we don't panic on bad input
1 reply
but always returning Err even for valid input would be invisible to the fuzzer
HeroicKatora
@heroic_katora:matrix.org
[m]
As an experience report the turnaround will be slow and fast, depending on mood 😂
It's also far more likely that Issues are opened for erroneous errors than for pathological cases.
And we can more easily throw a fuzzer run in than run against the vastness of images on the internet.
5225225
@r522:matrix.org
[m]
heads up HeroicKatora : ravif 0.7.0 has been yanked so cargo check on a clean checkout of image-rs/image doesn't build, bumping it to 0.8.0 looks to work fine
can make a PR to do that bump if you want
HeroicKatora
@heroic_katora:matrix.org
[m]
Sure, feel free to do.
Do you happen to know why it was yanked? Not that it really matters if the bump is a clean one to do