hyperrealm / libconfig

C/C++ library for processing configuration files
https://hyperrealm.github.io/libconfig/
GNU Lesser General Public License v2.1
1.1k stars 360 forks source link

int64 issues #153

Open kstreitova opened 4 years ago

kstreitova commented 4 years ago

Hello,

2 years back one of the SUSE engineers found that libconfig has a series of issues when supporting and parsing 64-bit integer values. It was supposedly reported to upstream by him but as he is no longer in SUSE, we can't evaluate if there is any progress in this matter. So I'm reporting it again so we are sure that it reached the upstream.

See https://gitlab.com/mcgrof/libconfig-int64-issues for more information.

Can you please evaluate?

Thank you!

hyperrealm commented 4 years ago

Not supporting unsigned integer types was a conscious decision, so as to improve compatibility with programming languages that do not have unsigned types (e.g, Java). Adding support for unsigned types would just shift the problem to such non-C/C++ format-compatible implementations.

The issue with the theoretical limit of 2^31 child settings per parent setting is noted, but I really do not anticipate anyone will be creating configuration files with billions of settings; that's just not practical, and not the intended use case for this library.

Checking for enormous file sizes before initiating parsing is outside the scope of this library, in my opinion. This library is intended to be small and focused, and such edge-case validations should be done by the calling application.

The int<->int64 casting issues and value rollovers are legitimate bugs to be addressed in libconfig. I'll look into these as soon as time permits.

danyspin97 commented 3 years ago

Any news on this issue?

acipm commented 1 year ago

Hi, are there any updates on this? I could not find a commit that addresses these issues and the patch from the gitlab repo that @kstreitova linked seems to be not applied as well. Will this still be addressed? Thank you very much for your work!

haydaralaidrus commented 1 year ago

The int<->int64 casting issues and value rollovers are legitimate bugs to be addressed in libconfig. I'll look into these as soon as time permits.

Is this issue has been raised before? How do I find a way to reproduce it?

hyperrealm commented 1 year ago

I am downgrading this to a feature request to add unsigned integer types.

I have tested with the inputs provided with this issue and I do not see an actual bug in libconfig. In no case does it return a clipped or wrapped or otherwise incorrect value. Values outside the supported range for a signed 64-bit integer result in a syntax error when reading a config file. A 'lookup' call to read a value into an int that is actually a 64-bit int which has a value outside the range of an int, result in an error return status from that call.

The 'L' suffix, and the handling of integers in general, is well documented in the libconfig manual. The library by design does not support unsigned integers or a 'U' suffix. Programs that expect setting values to be within a specific range (for example, non-negative), have to range-check those values themselves.

It's possible that some of the issues mentioned here were an issue at one point and have since been fixed.