Lots of implicit integer type conversion errors

[workbbraun]

Internally, it appears libxo is essentially only using two integer types, 'int' and 'unsigned'. Both of these are frequently 32bits even on 64bit systems.
Throughout libxo, pointer arithmetic is being done and storing the value in 'int' types. Nearly all of libxo/xo_buf.h's inline functions are this way.
Buffer sizes, lengths, and positive-only offsets should be size_t, and offsets that need negative values should be ssize_t.
Unfortunately, this affect nearly every type and function in libxo, including its API. Even the formatter function that is ostensibly derived from vsnprintf uses an int for buffer size instead of vsnprintf's size_t. Similarly, write() derived functions operate on ints instead of size_t and ssize_t.
This affects the internal wrapper functions xo_strn* that take a -1 and then do a strlen() on the input, since all strn* functions properly take a size_t for the buffer size, but the wrappers take an int in order to accommodate the -1 behavior.
There seems to be a lot of potential for vulnerabilities in just the integer type conversions and sign mismatches.

[philshafer]

Looks like the only places this would be exposed as an API change are in the typedefs of the three callback functions, xo_formatter_t, xo_checkpointer_t, and xo_simplify_field_func_t. The first two are using for external encoders and the last for custom i18n. These are all rarely used, which should minimize the impact of the fix.

Thanks,
Phil

On Oct 26, 2016, at 6:39 PM, workbbraun notifications@github.com wrote:

Internally, it appears libxo is essentially only using two integer types, 'int' and 'unsigned'. Both of these are frequently 32bits even on 64bit systems.
Throughout libxo, pointer arithmetic is being done and storing the value in 'int' types. Nearly all of libxo/xo_buf.h's inline functions are this way.
Buffer sizes, lengths, and positive-only offsets should be size_t, and offsets that need negative values should be ssize_t.
Unfortunately, this affect nearly every type and function in libxo, including its API. Even the formatter function that is ostensibly derived from vsnprintf uses an int for buffer size instead of vsnprintf's size_t. Similarly, write() derived functions operate on ints instead of size_t and ssize_t.
This affects the internal wrapper functions xo_strn* that take a -1 and then do a strlen() on the input, since all strn* functions properly take a size_t for the buffer size, but the wrappers take an int in order to accommodate the -1 behavior.
There seems to be a lot of potential for vulnerabilities in just the integer type conversions and sign mismatches.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #55, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLkOPgBZ3w9spM3VeC4WBz0sly5MIIDks5q39aqgaJpZM4KhwJT.

[philshafer]

Apologies. Sloppiness on my part. Will address these in the next release, which will require an API change.

Thanks,
Phil

On Oct 26, 2016, at 6:39 PM, workbbraun notifications@github.com wrote:

Internally, it appears libxo is essentially only using two integer types, 'int' and 'unsigned'. Both of these are frequently 32bits even on 64bit systems.
Throughout libxo, pointer arithmetic is being done and storing the value in 'int' types. Nearly all of libxo/xo_buf.h's inline functions are this way.
Buffer sizes, lengths, and positive-only offsets should be size_t, and offsets that need negative values should be ssize_t.
Unfortunately, this affect nearly every type and function in libxo, including its API. Even the formatter function that is ostensibly derived from vsnprintf uses an int for buffer size instead of vsnprintf's size_t. Similarly, write() derived functions operate on ints instead of size_t and ssize_t.
This affects the internal wrapper functions xo_strn* that take a -1 and then do a strlen() on the input, since all strn* functions properly take a size_t for the buffer size, but the wrappers take an int in order to accommodate the -1 behavior.
There seems to be a lot of potential for vulnerabilities in just the integer type conversions and sign mismatches.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #55, or mute the thread https://github.com/notifications/unsubscribe-auth/ACLkOPgBZ3w9spM3VeC4WBz0sly5MIIDks5q39aqgaJpZM4KhwJT.

[philshafer]

Fixed in 0.7.2.