Page MenuHomePhabricator

exactness: handbuild a new argv array instead of reassemling the new one
ClosedPublic

Authored by bu5hm4n on Mar 25 2020, 7:31 AM.

Details

Summary

this commit removes the code that was changing argv values, and replaces
it with a new array. Which is absolutly fine, as the argv / argc values
are never accessed later on. Only the copies that have been passed to
efl_main or elm_main.

This resolves several issues:

  1. the for loop is useless, every single array element that gets initialized with it, is some offset from argv[0] this may also crash when argv[i] - argv[opt_args] is bigger strlen argv[0].
  2. The memcpy here is super dangerous, the dest array is not garanteed to have the same size as argv[0], this only works if the client application name is shorter than the name "exactness_recorder"
  3. The memset here is absolutly wrong. There is again no garantee that the array has the expected size behind that, this was constantly overwriting the segment after the place where argv was stored, which was lukely enough on fedora always the environs, which deleted the couple first segments. (This was not causing any fuzz, since they have been sudo related env vars on the docker image). However, on arch this just crashed right away. On Ubuntu this overwrote DISPLAY, which resulted in the unability to launch the app.

Diff Detail

Repository
rEFL core/efl
Branch
master
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 16437
bu5hm4n created this revision.Mar 25 2020, 7:31 AM

It seems that this patch has no reviewers specified. If you are unsure who can review your patch, please check this wiki page and see if anyone can be added: https://phab.enlightenment.org/w/maintainers_reviewers/

bu5hm4n requested review of this revision.Mar 25 2020, 7:31 AM
bu5hm4n updated this revision to Diff 29692.Mar 25 2020, 7:41 AM
bu5hm4n edited the summary of this revision. (Show Details)

better commit message, git add -p accident

stefan_schmidt requested changes to this revision.Mar 25 2020, 8:08 AM
stefan_schmidt added a subscriber: stefan_schmidt.

We have a very similar construct in player.c line 1086ff it would be good to rool the change for it in here as well.

The rest looks good and much saner now. :-)

This revision now requires changes to proceed.Mar 25 2020, 8:08 AM
bu5hm4n updated this revision to Diff 29693.Mar 25 2020, 9:07 AM

add exactness_player

bu5hm4n updated this revision to Diff 29694.Mar 25 2020, 9:11 AM

add correct field

This revision is now accepted and ready to land.Mar 25 2020, 9:58 AM
Closed by commit rEFL66e2d7141471: exactness: handbuild a new argv array instead of reassemling the new one (authored by Marcel Hollerbach <mail@marcel-hollerbach.de>). · Explain WhyMar 25 2020, 2:14 PM
This revision was automatically updated to reflect the committed changes.