Perl and eval, not what I'd have expected from the title!

Indeed! Arbitrary text was smuggled into the eval() statement through a weak input sanitization step:

The second quote was not escaped because in the regex $tok =~ /(\\+)$/ the $ will match the end of a string, but also match before a newline at the end of a string, so the code thinks that the quote is being escaped when it’s escaping the newline.

This is why I hate using regexes, or deciphering other people's code which uses them. The syntax just isn't obvious. We need tooling that removes mental overhead, not adds to it. I've always found plain old procedural parsing code easier to formulate, trace and reason about.

I'm not some some fanatic preaching they should be banned from all languages, but I do feel there are places they're inappropriate and input sanitization is one of them. It's also good practice to centralize this sort of logic in some flavor of escape() function to reduce the whack-a-mole when bugs like this are found.

"will match the end of a string, but also match before a newline at the end of a string."

Is it just me, or is this a poor description? I understood $ to be an anchor for end of line, not end of string, i.e. "match before a newline anywhere". The emphasis "newline at the end of a string" feels misleading; the exploit works because the matched newline isn't at the end of the string and there is exploit code after it.

"I've always found plain old procedural parsing code easier to formulate, trace and reason about."

A regex is a domain specific language which lets you express a lot of computation in a small amount of code. At the extreme, you're saying "I can write a regex engine easier than I can write a regex". "I can write assembler easier than I can write C". "I can write a list of a thousand items quicker than I can write range(1000)". It doesn't make sense, and for anything more than trivial cases, I don't believe it.

(I'll agree that there are places they are inappropriate, expressions which aren't clear, and maybe the extra effort of doing it by hand is worth it in security sensitive situations, but I claim it is extra effort and less clear what a pile of ifs/loops/switch is trying to achieve vs "[a-f]{3}(\d)" or whatever).

The meaning of ^ and $ in perl regexes can be altered by modifiers at the end of the regex. A 'multi-line' regex, with a /m at the end, makes them match the start and end of any line.

The code doesn't use /m. So indeed the problem is that $ will match before \n at the end of the string the Perl interpreter is working with, which is not the whole thing containing the payload.

So how do I know if there is bug bounty available for vulnerabilities in exiftool? Or ghostscript? Or ffmpeg, openssl, gnutls, sox, or any number of other packages I may be using?

They're open source and written by volunteers so there almost certainly isn't. Unless it's covered by this:

https://bughunters.google.com/about/rules/6521337925468160/g...

This was fixed in version 12.24 (back in 2021) according to the version history page[0], but the current version still uses "eval" in several places[1]. This seems like an unnecessarily dangerous approach – wouldn't it have been a good idea to fix all the instances in the codebase after the bug was discovered?

[0] https://exiftool.org/ancient_history.html#v12.24

[1] https://github.com/search?q=repo%3Aexiftool%2Fexiftool+%2Fev...

Indeed. Also I wonder if Gitlab is still using this.

Gitlab self-hosted users who didn't patch got bitten with in the wild exploitation: https://www.rapid7.com/blog/post/2021/11/01/gitlab-unauthent...

From TFA:

In Perl, eval can be used with a block to trap exceptions which is why it was being used everywhere.

...which your search doesn't seem to exclude (I'm not signing up to Github to find out so I'm just guessing from your URL that you didn't exclude "eval {" or "eval q{")

It does still use eval on occasion and they seem happy with it.

* for their own controlled expressions to customise base parsing behaviour (which could be refactored to use function references instead, but it's not a case of evaluating external data)

* to test if modules can be imported (which you've excluded from your search)

and in one case in CanonRaw.pm, using eval on an expression that matches [0-9./]+ , I'm not sure it looks at external data but it might, and at best you could cause a divide-by-zero error (e.g. 1/0) or syntax error (e.g. ///) if you mess with the data it parses.

So overall, not much dangerous eval going on.

Catching exceptions from eval-ing untrusted code doesn't strike me as being a big help from security POV, or is there something else special about the block form of eval that helps here?

The non-GH search version: https://sourcegraph.com/search?q=context:global+repo:%5Egith...

Your observation is proper in that it did not exclude the patterns you mentioned, but in this specific case not necessary because all 4 results are shaped the exact same way:

    lib/Image/ExifTool/Exif.pm

                    #### eval Start ($valuePtr, $val)
                    my $newStart = eval($$subdir{Start});
                    unless (Image::ExifTool::IsInt($newStart)) {
                    #### eval Base ($start,$base)
                    $subdirBase = eval($$subdir{Base}) + $base;
                }

    lib/Image/ExifTool/MakerNotes.pm

                #### eval Start ($valuePtr)
                $newStart = eval($$subdir{Start});
            }
                #### eval Base ($start,$base)
                my $baseShift = eval($$subdir{Base});
                # shift directory base (note: we may do this again below
                #### eval OffsetPt ($valuePtr)
                $ifdOffsetPos = eval($$subdir{OffsetPt}) - $dirStart;
            }

I prefer Exiv2 over ExifTool. I can compile it statically. Does not require a Perl installation.