swupdate-progress: make sure bar array is 0-terminated (buffer overflow)

[tthef]

The bar filling was based on the assumption that there are sizeof(bar) - 1 characters, but terminating 0 was never set. The bar array was 60 chars, and the fprintf formatting string expected to find 60 chars, thus printing one garbage char, and the missing 0-terminator was causing the app to crash.

The commit increases the size of bar by 1, and 0-terminates it.

Signed-off-by: Tomas Frydrych [email protected]

[bgermann]

I guess you should send this to the mailing list.

[tthef]

Sorry, not joining yet another mailing list to submit patches.

[bgermann]

I do not think you need to join.

[MarkJonas]

~~I think the patch has a problem: When increasing the size of bar by one with char bar[61] also sizeof(bar) will increase by one. Then the maximum value of filled_len = sizeof(bar) * msg.cur_percent / 100 will also be 61.~~

~~Next memset(bar,'=', filled_len) will still use the last byte which should be reserved for the terminating 0. The added bar[sizeof(bar)-1] = 0 will then overwrite it and this is why the code does not crash for you any more.~~

~~So the patch is on the right way but needs to be a little more sophisticated.~~

EDIT: It seems the code is already too sophisticated for me and I got lost. After staring at the code for a little longer I think it does the right thing.

IMHO it would help the readability if instead of sizeof(bar) and sizeof(bar)-1 a straight forward const int bar_len=60 would be used. But that's a different story.

EDIT 2: After even more thinking the truth might be in the middle:

With the proposed increase of char bar[60] to char bar[61] the progress bar scaling by filled_len = sizeof(bar) * msg.cur_percent / 100 will have a maximum of 61. This is then cut down to 60 by the following if (filled_len > sizeof(bar) - 1) filled_len = sizeof(bar) - 1. So everything is safe but the progress bar will stall for the last = character.

IMHO introducing a const int bar_len=60 will solve this problem and increase the readability of the code.

[MarkJonas]

The reported problem has been been fixed in -master. Please see https://github.com/sbabic/swupdate/commit/609ad51f581b92008c5fcaf1e5783d4e313f18cb .

@tthef You might want to close this PR now.

[MarkJonas]

The reported problem has been been fixed in -master. Please see 609ad51 .

@tthef You might want to close this PR now.

@sbabic @tthef I propose to close the PR.