Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Argtable3] stack corruption for long arguments (IDFGH-8566) #10016

Conversation

chipweinberger
Copy link
Contributor

@chipweinberger chipweinberger commented Oct 20, 2022

EDIT: MERGED into argtable directly: argtable/argtable3@f25c624

For large command line arguments, 200 chars or greater, arg_cat writes out of bounds.

/api/auth/upload-public-keys: excess option 
4ac8821c6f79d5d06092a864886f70d4bd83d81ca1e50f08288df9034964d380171dda6d1d7dd73ecblecdd343424da87d6a65d21775e9
3082094202010030001010105000482092c308200002820201009280201587ba727d9a5c3eae900106827a4211

Stack smashing protect failure!

abort () was called at PC 0x42004d2f on core 1

0x42004d2f: stack chk fail at /esp-idf/components/esp_system/stack_check.c:36

0x40376473: panic abort at /esp-idf/components/esp_system/panic.c:455

0x40384a49: esp_system_abort at /esp-idf/components/esp_system/esp_system.c:128

0x4038fad5: abort at /esp-idf/components/newlib/abort.c:46

0x42004d2f: _ stack chk fail at /esp-idf/components/esp_system/stack_check.c:36

0x420e9025: arg_print _option at /esp-idf/components/console/argtable3/argtable.c:4527

0x420e9096: arg_str_errorfnat/esp-idf/components/console/argtable3/argtable3.c:3679

0x42082708: arg_print_errors at /esp-idf/components/console/argtable3/argtable3.c:1768

0x42011816: cd standard handler api isonTo]son(int, char**) at /common/api/coms/pd console handlers. cpp:306

0x420809d: esp console run at /esp-idf/components/console/commands.c:196

ommon/api/xcoms/p console task.cpp: 392
®x40389f4e: vPortTaskWrapper at /esp-idf/components/freertos/port/xtensa/port.c:131
ELF file SHA256: c881812bac619916

@chipweinberger chipweinberger force-pushed the user/chip/argtable-string-cat-overflow branch from 770b7fd to 3df5050 Compare October 20, 2022 03:22
dest++;

/* concat src string to dest string */
while (dest < end && *src != 0)
while (dest < (end-1) && *src != 0)
*dest++ = *src++;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

end is past the end of the buffer. end = dest + buffer_size.

i.e. if end is 0x05, the last time would should enter the loop (& increment dest) is when dest is 0x03.

last iteration: 0x03 < 0x04 -> dest will become 0x04

@espressif-bot espressif-bot added the Status: Opened Issue is new label Oct 20, 2022
@github-actions github-actions bot changed the title [Argtable3] stack corruption for long arguments [Argtable3] stack corruption for long arguments (IDFGH-8566) Oct 20, 2022
@igrr
Copy link
Member

igrr commented Oct 20, 2022

Thanks for the fix @chipweinberger!
Could you please send the PR to the upstream project? We will be updating from upstream soon since there was another fix recently merged there.

@chipweinberger chipweinberger force-pushed the user/chip/argtable-string-cat-overflow branch from 3df5050 to 7915973 Compare October 20, 2022 07:30
@chipweinberger
Copy link
Contributor Author

PR: argtable/argtable3#78

@tomghuang
Copy link

@chipweinberger : Thanks for the PR. I'll review it as soon as possible.

@igrr : After reviewing and merging this patch, I'll create a new release, since this fix is critical. I'll let you know when the release is ready.

@tomghuang
Copy link

@igrr : I've published a new release (v3.2.2) that included this patch. Thanks.

@chipweinberger
Copy link
Contributor Author

Appreciate your service!

@igrr
Copy link
Member

igrr commented Oct 24, 2022

@tomghuang Thank you for accepting the PR and making the release!

@chipweinberger Thank you for your contribution! We'll upgrade argtable to the new release soon.

@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Opened Issue is new Resolution: NA Issue resolution is unavailable labels Oct 27, 2022
espressif-bot pushed a commit that referenced this pull request Nov 11, 2022
@chipweinberger chipweinberger deleted the user/chip/argtable-string-cat-overflow branch November 18, 2022 23:47
espressif-bot pushed a commit that referenced this pull request Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants