(add-time ) overflow => unexpected results (copied from Pact Legacy)

[CryptoPascal31]

Hey,
I'm sorry to insist... I copied this issue from Pact Legacy... Because in my opinion, this one is the most critical "not already fixed" issue in Pact legacy. It's not a nice to have, neither an edge case: it's definitively a security issue. I can easily write a vulnerable contract. (can show an example if requested).

Maybe the release of Pact core is the good timing to fix it.
FYI: I created a "a la Solidty" workaround: (not published and only very quickly tested): https://github.com/CryptoPascal31/pact-util-lib/blob/ac97feca2dcf1176d9bcc55510a2964dd9d663f6/pact/contracts/util-time.pact#L46

But IMHO, it should be definitively been fixed in Haskell. Because having a workarond in Pact is not an acceptable solution.

Issue description

The add-time function can overflow and give unexpected results.

Steps to reproduce

Some examples:

pact> (add-time (time "2016-07-22T12:00:00Z") 10000000000000000000000000000000000)
"-157918-10-27T19:13:49Z" 

pact> (add-time (time "2016-07-22T12:00:00Z") 1000000000000000000000000000000000000000000000000000000000000000000000000)
"2016-07-22T12:00:00Z"

As you can see in some cases:

• The time wraps around
• The function doesn't work at all and returns the original time.

Expected Behavior

Either:

• The function should trigger a transaction failure when an overflow is expected.
• In the documentation, flag the add-time as "unsafe" without prior boundaries check.

[jmcardon]

Thank you so much for the report.

We've actually raised this issue internally before, and the fix is on the roadmap. Thanks for bringing it to the forefront.

Indeed, pact-5 doesn't have it fixed because we're using the exact same library as pact (pact-time), so unfortunately, we have to fix it in the library or alternatively add a runtime check directly in the new compiler.

[CryptoPascal31]

Thank you so much for the report.

We've actually raised this issue internally before, and the fix is on the roadmap. Thanks for bringing it to the forefront.

Indeed, pact-5 doesn't have it fixed because we're using the exact same library as pact (pact-time), so unfortunately, we have to fix it in the library or alternatively add a runtime check directly in the new compiler.

IMHO, the fix is relatively easy... It's just a matter of enforcing safe boundaries for dates and deltas, for unrealistic dates... (year 147997)

See my solution I did in Pact (I added some useful comments) . but need to be carefully reviewed and evaluated