haskell / alex

A lexical analyser generator for Haskell
https://hackage.haskell.org/package/alex
BSD 3-Clause "New" or "Revised" License
298 stars 82 forks source link

Fix out-of-bound access to "alex_check" array #223

Closed hsyl20 closed 1 year ago

hsyl20 commented 1 year ago

offset can be negative. We must check this before using it to index into alex_check array.

Caught with GHC's JS backend (native GHC would only segfault in rare circumstances).

andreasabel commented 1 year ago

@Ericson2314 : Could you please add me to the maintainers: https://hackage.haskell.org/package/alex/maintainers/

andreasabel commented 1 year ago

Candidate: https://hackage.haskell.org/package/alex-3.2.7.2/candidate

Published: https://hackage.haskell.org/package/alex-3.2.7.2

hsyl20 commented 1 year ago

@andreasabel Awesome. Thanks a lot for your responsiveness!

RyanGlScott commented 1 year ago

One consequence of this PR is that alex-generated .hs files now require PatternGuards, which wasn't required before:

$ ghc -XHaskell98 Foo.hs
[1 of 1] Compiling Main             ( Foo.hs, Foo.o )

Foo.hs:3747:19: warning:
    accepting non-standard pattern guards (use PatternGuards to suppress this message)
        (offset >= (0)),
        check <- alexIndexInt16OffAddr alex_check offset,
        (check == ord_c)
     |
3747 |                   | GTE(offset,ILIT(0))
     |                   ^^^^^^^^^^^^^^^^^^^^^..

In fact, I had a project break with alex-3.2.7.2 because of this. Perhaps it would be worth documenting the PatternGuards requirement somewhere?

andreasabel commented 1 year ago

@hsyl20 : Can this be implemented without PatternGuards? How about this:

 new_s = if GTE(offset,ILIT(0)) 
                && let check = alexIndexInt16OffAddr alex_check offset in EQ(check,ord_c)
                          then alexIndexInt16OffAddr alex_table offset
                          else alexIndexInt16OffAddr alex_deflt s
andreasabel commented 1 year ago

See: