botan icon indicating copy to clipboard operation
botan copied to clipboard

Incorrect help text for stack protection option in configure.py

Open pkl97 opened this issue 4 months ago • 3 comments

configure.py in Botan 3.9.0 displays the following text: --with-stack-protector disable stack smashing protections

Shouldn't it be "enable stack smashing protections"?

pkl97 avatar Sep 04 '25 07:09 pkl97

What's happening now is certainly incorrect. I think what it really should be displaying is --without-stack-protector disable ... since with is the default.

randombit avatar Sep 04 '25 12:09 randombit

Hello,

I would like to take on this error case (even though I don't have much experience with Python). I haven't had time to look into it in detail yet, but I thought it might be useful to share my thoughts.

Here's the solution I'm thinking of: If I change the default value to True, the message output is generated correctly. However, in the current situation, when the value is None, the configurations under botan/src/os/* are taken into account. If we set the default directly to True, it could lead to this value being incorrectly assumed to be enabled on unsupported platforms (AIX, Emscripten etc.). Note: The default is 'True', even though the parameter is 'None' on other platforms.

Alternatively, extending the add_with_without_pair function could be considered. However, since I don't have sufficient knowledge about this file, I didn't think it appropriate to modify it directly. Therefore, I plan to remove the existing line and add the following lines instead:

Line to be removed:

add_with_without_pair(build_group, ‘stack-protector’, None, ‘control stack smashing protections’)

Lines to be added:

build_group.add_option(‘--with-stack-protector’, dest=‘with_stack_protector’,
                      action=‘store_true’, default=None,
                      help=‘enable stack smashing protections’)

build_group.add_option(‘--without-stack-protector’, dest=‘with_stack_protector’,
                      action=‘store_false’, default=None,
                      help=‘disable stack smashing protections’)

This solution creates some code duplication. Maybe a better method be found. However, proceeding this way seems feasible in the initial phase. I can contribute with a little more information and guidance.

I haven't completed the detailed review yet; I'll take another look.

KaganCanSit avatar Sep 04 '25 20:09 KaganCanSit

The root cause is that we don't know the platform-default when registering this option. Currently None is passed as a placeholder which will be rectified at a later stage (in canonicalize_options) after the platform-default is known.

The displayed switch (--with-stack-protector) doesn't match the help-text ("disable stack smashing protection"), because the generic add_with_without_pair falsely treats the None default as False.

I think we should consider loading the platform-config files before rendering the command-line options. With that, we could display --without-stack-protector if the platform-default is "with" and vise-versa. We could even not display the option at all, if the compiler info doesn't provide stack protector flags at all.

reneme avatar Sep 05 '25 07:09 reneme