{"action":"created","issue":{"url":"https://api.github.com/repos/spacetelescope/hstcal/issues/150","repository_url":"https://api.github.com/repos/spacetelescope/hstcal","labels_url":"https://api.github.com/repos/spacetelescope/hstcal/issues/150/labels{/name}","comments_url":"https://api.github.com/repos/spacetelescope/hstcal/issues/150/comments","events_url":"https://api.github.com/repos/spacetelescope/hstcal/issues/150/events","html_url":"https://github.com/spacetelescope/hstcal/issues/150","id":234611508,"number":150,"title":"Buffer overflow in strcpy called from calwf3","user":{"login":"jamienoss","id":5013975,"avatar_url":"https://avatars1.githubusercontent.com/u/5013975?v=3","gravatar_id":"","url":"https://api.github.com/users/jamienoss","html_url":"https://github.com/jamienoss","followers_url":"https://api.github.com/users/jamienoss/followers","following_url":"https://api.github.com/users/jamienoss/following{/other_user}","gists_url":"https://api.github.com/users/jamienoss/gists{/gist_id}","starred_url":"https://api.github.com/users/jamienoss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamienoss/subscriptions","organizations_url":"https://api.github.com/users/jamienoss/orgs","repos_url":"https://api.github.com/users/jamienoss/repos","events_url":"https://api.github.com/users/jamienoss/events{/privacy}","received_events_url":"https://api.github.com/users/jamienoss/received_events","type":"User","site_admin":false},"labels":[{"id":341367902,"url":"https://api.github.com/repos/spacetelescope/hstcal/labels/ACS","name":"ACS","color":"bfd4f2","default":false},{"id":328428004,"url":"https://api.github.com/repos/spacetelescope/hstcal/labels/bug","name":"bug","color":"fc2929","default":true},{"id":341367995,"url":"https://api.github.com/repos/spacetelescope/hstcal/labels/WFC3","name":"WFC3","color":"bfd4f2","default":false}],"state":"open","locked":false,"assignee":{"login":"jamienoss","id":5013975,"avatar_url":"https://avatars1.githubusercontent.com/u/5013975?v=3","gravatar_id":"","url":"https://api.github.com/users/jamienoss","html_url":"https://github.com/jamienoss","followers_url":"https://api.github.com/users/jamienoss/followers","following_url":"https://api.github.com/users/jamienoss/following{/other_user}","gists_url":"https://api.github.com/users/jamienoss/gists{/gist_id}","starred_url":"https://api.github.com/users/jamienoss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamienoss/subscriptions","organizations_url":"https://api.github.com/users/jamienoss/orgs","repos_url":"https://api.github.com/users/jamienoss/repos","events_url":"https://api.github.com/users/jamienoss/events{/privacy}","received_events_url":"https://api.github.com/users/jamienoss/received_events","type":"User","site_admin":false},"assignees":[{"login":"jamienoss","id":5013975,"avatar_url":"https://avatars1.githubusercontent.com/u/5013975?v=3","gravatar_id":"","url":"https://api.github.com/users/jamienoss","html_url":"https://github.com/jamienoss","followers_url":"https://api.github.com/users/jamienoss/followers","following_url":"https://api.github.com/users/jamienoss/following{/other_user}","gists_url":"https://api.github.com/users/jamienoss/gists{/gist_id}","starred_url":"https://api.github.com/users/jamienoss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamienoss/subscriptions","organizations_url":"https://api.github.com/users/jamienoss/orgs","repos_url":"https://api.github.com/users/jamienoss/repos","events_url":"https://api.github.com/users/jamienoss/events{/privacy}","received_events_url":"https://api.github.com/users/jamienoss/received_events","type":"User","site_admin":false}],"milestone":null,"comments":3,"created_at":"2017-06-08T18:10:05Z","updated_at":"2017-06-27T14:04:25Z","closed_at":null,"body":"When running calw3.e on iacr51ojq_raw.fits (on OSX) in the terminal it completes without fault, however, when running with GDB it aborts [here](https://github.com/spacetelescope/hstcal/blob/master/pkg/wfc3/calwf3/calwf3/wf3member.c#L106) in ``calwf3/wf3members.c``.\r\n\r\n```c\r\nint GetSingle (AsnInfo *asn, WF3Info *wf3) {\r\n...\r\nstrcpy(outroot, asn->filename);\r\n...\r\n```\r\n\r\nThis is due to ``outroot`` being of length ``SZ_CBUF`` := 24 ([here](https://github.com/spacetelescope/hstcal/blob/master/pkg/wfc3/calwf3/include/msg.h#L4)) when it should really be of length ``SZ_FNAME`` (in this instance ``strlen(asn->filename)`` = 62). However, it seems to me like the entire use of outroot is incorrect in [int GetSingle (AsnInfo *asn, WF3Info *wf3)](https://github.com/spacetelescope/hstcal/blob/master/pkg/wfc3/calwf3/calwf3/wf3member.c#L95-L134)\r\n\r\n```c\r\nint GetSingle (AsnInfo *asn, WF3Info *wf3) {\r\n\r\n/* arguments:\r\nAsnInfo *asn      \ti: calibration flags and other info\r\nWF3Info *wf3\t\to: exposure specific flags and info\r\n*/\r\n\textern int status;\r\n\tchar rootname[SZ_FNAME+1];\r\n\tchar outroot[SZ_CBUF+1];\r\n\tvoid FindAsnRoot (char *, char *);\r\n\t\r\n\tstrcpy(outroot, asn->filename);\r\n\t\r\n\t/* Make sure we are only passing a rootname, and not a full filename.*/\r\n\tFindAsnRoot (outroot, rootname);\r\n\tstrcpy (wf3->outroot, rootname);\r\n\tstrcpy (wf3->rootname, rootname);\r\n\t\r\n\tif (asn->debug) {\r\n\t    sprintf (MsgText, \"GetSingle: Rootname: %s, Output rootname: %s\",\r\n\t\t     rootname, outroot);\r\n\t    trlmessage (MsgText);\r\n\t}\r\n\r\n\t/* Check to see that this value of rootname is what we really need. */\r\n\tstrcpy (wf3->asn_table, asn->asn_table);\t\r\n\tstrcpy (wf3->rawfile, asn->filename);\r\n\t\r\n\t/* Set sci_* flags for wf3 */\t\r\n\twf3->sci_crcorr  = asn->crcorr;\r\n\twf3->sci_dthcorr = asn->dthcorr;\r\n\twf3->sci_rptcorr = asn->rptcorr;\r\n\twf3->detector    = asn->detector;\r\n\twf3->nimages     = 1;\r\n\r\n\t/* Set MemType appropriate for output */\r\n\twf3->mtype[0] = '\\0';\r\n\r\n\treturn (status);\r\n}\r\n```\r\n\r\nSo, ``FindAsnRoot`` doesn't really find the actual root, i.e. dir path to file or root dir for entire path, but instead the entire thing including the filename, so i.e. it just removes the suffix (and only a certain set of suffixes at that - also they're not really file sufixes either but '_blv', '_flc' etc). In ``void FindAsnRoot (const char * input, char * output);`` ``char * input`` should be ``const char * input``.\r\n\r\nAnyway, ``outroot`` isn't actually used other than in a print statement so should be removed or at least made just a reference. Secondly, a copy is made of it within ``FindAsnRoot()``.\r\n\r\nIn addition, ``wf3::outroot`` is actually set to ``rootname`` ([here](https://github.com/spacetelescope/hstcal/blame/master/pkg/wfc3/calwf3/calwf3/wf3member.c#L110)) and not ``outroot`` as it probbaly should.\r\n\r\n```c\r\nstrcpy (wf3->outroot, rootname);\r\n```\r\n\r\nSo, even with this all fixed, it then aborts on the copy of ``strcpy (wf3->rootname, rootname);`` because of the length of ``WF3Info::rootname[SZ_CBUF+1]``. This should either be the length of ``SZ_LINE+1`` as ``WF3Info::outname`` is (it is in [ACSInfo](https://github.com/spacetelescope/hstcal/blob/master/pkg/acs/calacs/include/acsinfo.h#L27)) _OR_ ``FindAsnRoot()`` really is wrong and really should be finding the canonical notion of a file root(?).\r\n\r\nI have no idea why this is not aborting in the terminal, especially since``--fstack-protector-all`` is set at compile time.\r\n\r\nAll of the above is true for ACS code also."},"comment":{"url":"https://api.github.com/repos/spacetelescope/hstcal/issues/comments/311368366","html_url":"https://github.com/spacetelescope/hstcal/issues/150#issuecomment-311368366","issue_url":"https://api.github.com/repos/spacetelescope/hstcal/issues/150","id":311368366,"user":{"login":"jamienoss","id":5013975,"avatar_url":"https://avatars1.githubusercontent.com/u/5013975?v=3","gravatar_id":"","url":"https://api.github.com/users/jamienoss","html_url":"https://github.com/jamienoss","followers_url":"https://api.github.com/users/jamienoss/followers","following_url":"https://api.github.com/users/jamienoss/following{/other_user}","gists_url":"https://api.github.com/users/jamienoss/gists{/gist_id}","starred_url":"https://api.github.com/users/jamienoss/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamienoss/subscriptions","organizations_url":"https://api.github.com/users/jamienoss/orgs","repos_url":"https://api.github.com/users/jamienoss/repos","events_url":"https://api.github.com/users/jamienoss/events{/privacy}","received_events_url":"https://api.github.com/users/jamienoss/received_events","type":"User","site_admin":false},"created_at":"2017-06-27T14:04:25Z","updated_at":"2017-06-27T14:04:25Z","body":"Thinking about this further it looks as if the problem is also due to the differing (or use of) ``ACS_FNAME 162`` in calacs vs ``SZ_FNAME 255`` that is used in calwf3. Why anything would be 163 (+1) bytes long is beyound me. I believe this should just be 255. If we are worrying about the mem space for these strings then we are worrying about all the wrong things - and shouldn;t have made it 162 in the first place for alignment issues."}}
{"actor":{"display_login":"jamienoss"}}