search

amazon-ion / ion-c

A C implementation of Amazon Ion.
https://amazon-ion.github.io/ion-docs/
Apache License 2.0
166 stars 43 forks source link

Fix undefined behavior when using CLI's process command. #307

Closed nirosys closed 2 years ago

nirosys commented 2 years ago

Issue #, if available: None

Description of changes: When using the process subcommand of the Ion-C CLI the ion_cli_command_process function is called using NULL for output ION_STRING parameter. At the end of ion_cli_command_process as part of the function's cleanup it calls ion_cli_close_writer supplying the NULL output string, which then gets used in a call to ion_event_writer_close, supplying the value and length fields of the ION_STRING to capture the writer's output.

Since the output string is NULL, the dereferences to obtain the value and length fields are undefined behavior. ub-san identifies the issue when running a debug build like so:

$ echo '{foo: bar}' | tools/cli/ion process -f pretty -
/home/ubuntu/ion-c/tools/cli/cli.cpp:236:5: runtime error: member access within null pointer of type 'ION_STRING' (aka '_ion_string')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/ubuntu/ion-c/tools/cli/cli.cpp:236:5 in 
/home/ubuntu/ion-c/tools/cli/cli.cpp:236:5: runtime error: member access within null pointer of type 'ION_STRING' (aka '_ion_string')
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /home/ubuntu/ion-c/tools/cli/cli.cpp:236:5 in 
{
  foo: bar
}

This PR simply adjusts the call site to supply a NULL for each, if output is NULL. Since we're writing directly to stdout, the output type is IO_TYPE_CONSOLE, ensuring that nothing will be done with the output value pointer and length.

After the PR:

$ echo '{foo: bar}' | tools/cli/ion process -f pretty -
{
  foo: bar
}

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.