Don't read bytes beyond the end pointer in consOpen

[lagunoff]

In some cases consOpen reads one byte after the end pointer. I discovered it using ghcjs where in the implementation of ByteString there are runtime checks whether accessed bytes are inside the boundaries, this seem to fix the issue.

[tittoassini]

Thanks for this, I am working on a new release of 'flat' and will incorporate your changes.

Do you have any simple test that shows this issue?

[lagunoff]

No, it's very hard to show this problem in ghc, you have to allocate a ByteString at the end of memory segment so that reading next byte will cause a SEGFAULT, and maybe that won't even work I can only reproduce this probelm in GHCJS

[tittoassini]

Even a test that works only under ghcjs would be useful. Can you reproduce it systematically or just occasionally?

Il giorno mer 12 gen 2022 alle ore 00:48 Vladislav Lagunov < @.***> ha scritto:

No, it's very hard to show this problem in ghc, you have to allocate a ByteString at the end of memory segment so that reading next byte will cause a SEGFAULT, and maybe that won't even work I can only reproduce this probelm in GHCJS

— Reply to this email directly, view it on GitHub https://github.com/Quid2/flat/pull/26#issuecomment-1010477990, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAFRQNJFCZ244H45VAV5ITUVS6UBANCNFSM5LHHOQSA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

-- Pasqualino "Titto" Assini http://networkpolitics.svbtle.com http://quid2.org/

[lagunoff]

Not a test but I managed to get a minimal example that reliable reproduces this problem (using GHCJS). And problem only occurs when the input ByteString is constructed using createFromArrayBuffer, parsing a ByteString with same contents but constructed with Data.ByteString.pack won't raise this error (apparently in this case bounds check is not performed). Last fact I found about this problem is that the datatype has to have three or more constructors otherwise problem also doesn't happen.

{-# LANGUAGE DeriveGeneric #-}
{-# LANGUAGE DeriveAnyClass #-}
{-# LANGUAGE TypeApplications #-}
import Data.ByteString as BS
import Data.JSString
import Flat
import GHC.Generics
import JavaScript.TypedArray.ArrayBuffer
import qualified GHCJS.Buffer as Buffer

data Foo
  = Foo
  | Bar
  | Baz
  deriving (Show, Generic, Flat)

main :: IO ()
main = do
  -- >>> [1]
  print $ BS.unpack $ flat Foo
  -- >>> Right Foo
  print $ unflat @Foo $ flat Foo
  -- >>> Right Foo
  print $ unflat @Foo $ BS.pack [1]
  -- >>> RangeError: Offset is outside the bounds of the DataView
  -- at DataView.getUint16
  print $ unflat @Foo $ fromSingleByte 1

fromSingleByte :: Int -> ByteString
fromSingleByte = Buffer.toByteString 0 Nothing .
  Buffer.createFromArrayBuffer . fromSingleByte_js

foreign import javascript unsafe
  "var buf = new ArrayBuffer(1);\
  \var view = new Int8Array(buf);\
  \view[0] = $1;\
  \$r = buf;"
  fromSingleByte_js :: Int -> ArrayBuffer

consopen-counterexample.tar.gz

[tittoassini]

First of all, many thanks for your help in this matter and apologies for taking so long in addressing it.

Maybe you should have been a bit more forceful.

Rather than wasting time in writing a test case you should have simply told me:

"Bloody fool, can't you see that you are guilty of a classic one-off bug? I therefore direct and command you to immediately merge this fix and afterwards go hide your head in shame under the nearest rock!"

I am sure that I would have complied expeditiously :-)

best

[lagunoff]

Haha, Ok) I'm really happy to contribute to this library, I'm using it a lot as transport in fullstack haskell apps, it's a great tool. Thanks to you and other people who develops and maintains flat 👍!