|Anonymous | Login||2018-12-12 12:24 UTC|
|Main | My View | View Issues | Change Log | Docs|
|Viewing Issue Simple Details|
|ID||Category||Severity||Type||Date Submitted||Last Update|
|0001144||[1003.1(2008)/Issue 7] System Interfaces||Editorial||Clarification Requested||2017-06-14 14:31||2018-11-15 17:35|
|Priority||normal||Resolution||Accepted As Marked|
|Final Accepted Text||Note: 0004169|
|Summary||0001144: May fmemopen(buf, len, "r") modify the buffer|
If a memory stream is open in read-only mode, is the implementation allowed to modify the contents of the buffer? I can't find wording against that, but it would be rather odd if any implementation actually did it. If modifications are allowed, must they be restored when the stream is closed? (It should be obvious that fread() has to return the original contents, but nothing prevents the application from acccessing the buffer directly both while the stream exists and after it is closed.)
It would be nice to be able to do
parse_string(const char *s)
FILE *f = fmemopen((void*)s, strlen(s), "r")
if (!f) err...
without having to strdup() s, both for simplifying the code and for performance in case the string is huge.
Conversely, is the application allowed to modify the buffer while the stream exists, and are modification required to be visible by a subsequent fread on the stream?
Clarify what an implementation is allowed to do with the buffer in case the stream is read-only.
Clarify ownership rules and the effects of accessing the buffer directly while a stream exists using the same buffer.
The interfaces that might use the handle returned are expected to return EBADF when a write is attempted on a handle not opened for writing. See fputc() Errors for this. This does not prevent the application from mapping all or portions as writable memory for access by other interfaces or direct assignment, nor should it in my opinion for some scenarios. An Application Usage note may be in order that it is the application's responsibility to establish read only locks on the buffer, if the implementation supports this, before using fmemopen() if this is a concern.
What may be ambiguous, normatively, is whether implementations are permitted to free() or realloc() an application provided buffer that malloc() or calloc() has allocated before fclose() is called. Note mprotect() is not sufficient for this; it establishes the residency of memory pages, but not how the memory manager may make use of them subsequently in allocating free()d pages to other processes. While I don't see any easy way to make an exploit of this, I think it counts as a security hole.
Maybe I wasn't clear, but I'm not concerned about what might happen if the application calls fwrite() or similar on the handle. What concerns me is that nothing seems to prevent the implementation from doing something crazy like this in implementing a read-only memstream:
f->orig = buf;
f->buf = malloc(len);
f->len = len;
memcpy(f->buf, buf, len);
memset(buf, 0, len);
memcpy(f->orig, f->buf, f->len);
The last sentence was about what one can expect from code such as
char a = "xxx", b;
f = fmemopen(a, 3, "r");
fread(&b, 1, 1, f);
a = 'y';
fread(&b, 1, 1, f);
Is b now 'x' or 'y'? Or is the access to a forbidden while f is "bound" to the buffer?
Come to think of it: Obvious implementations of memstreams use memcpy to implement fread and fwrite, but is a call such as fread(&a, 1, 2, f) (with f just opened as above) allowed? Must the implementation support this and use memmove, or is it up to the application to avoid such calls?
But you also raise some valid points about the lifetime of the buffer; there should probably be some wording added about the application having to ensure that the buffer isn't deallocated before fclose().
edited on: 2017-06-14 23:14
I believe it does preclude this type of implementation, because the stream position is required to point into the buffer, not any buffer copy, in the paragraph at L29742. This means the implementation of fseek() has to store buf, position, and buf+size for the SEEK_* types, when for a file these would be 0, position, and size respectively. This could be clearer, I suppose. For the example I'd expect 'y', not 'x', stored in a and b after that sequence. I don't see the second example precluded as a result, and might be used in a strinsert() or fillpat() implementation.
Sorry for piling on, but storing an internal pointer into the original buffer doesn't prevent the implementation from playing these games. It is allowed to xor every byte of the original buffer with 0xab, undoing the effect of that on every fread() and finally on fclose(), AFAICT. Or to follow the letter of keeping a pointer into the original buffer, it can do that, but then just use pointer arithmetic to compute the offset it will actually read from in its internal copy.
I don't know any implementations that do play such silly games, nor can I think of any reason why they ever would, but still, I would very much like it spelled out that when a memstream is open read-only, the implementation is not allowed to modify the contents of the given buffer in any way.
I'll open seperate issues for the other points, since at least glibc doesn't behave as you (and I) expect in the a/b example above.
edited on: 2018-11-15 17:37
(All page and line numbers are for the 2016 edition)
On page 496 line 17235 section 2.5, change:
For output, data is moved from the buffer provided by setvbuf() to the memory stream during a flush or close operation.to:
For output, if the stream is fully buffered or line buffered, data shall be moved from the stream's internal buffer, or a buffer provided by setvbuf(), to the memory buffer during a flush or close operation. For input, it is unspecified whether a buffer provided by setvbuf() is used or whether read operations read directly from the memory buffer provided to or allocated by fmemopen().
On page 881 line 29741 section fmemopen(), add a new paragraph:
When a stream is opened for reading only and buf is not a null pointer, the buffer pointed to by buf shall not be modified by any operation performed on the stream.
On page 1931 line 62206 section setvbuf(), change the EBADF description from:
The file descriptor underlying stream is not valid.to:
The stream is not a memory stream and the file descriptor underlying the stream is not valid.
|2017-06-14 14:31||Villemoes||New Issue|
|2017-06-14 14:31||Villemoes||Status||New => Under Review|
|2017-06-14 14:31||Villemoes||Assigned To||=> ajosey|
|2017-06-14 14:31||Villemoes||Name||=> Rasmus Villemoes|
|2017-06-14 14:31||Villemoes||Section||=> fmemopen|
|2017-06-14 16:41||shware_systems||Note Added: 0003761|
|2017-06-14 19:35||Villemoes||Note Added: 0003762|
|2017-06-14 23:06||shware_systems||Note Added: 0003763|
|2017-06-14 23:14||shware_systems||Note Edited: 0003763|
|2017-06-22 07:23||Villemoes||Note Added: 0003796|
|2018-11-15 17:33||geoffclare||Note Added: 0004169|
|2018-11-15 17:35||geoffclare||Interp Status||=> ---|
|2018-11-15 17:35||geoffclare||Final Accepted Text||=> Note: 0004169|
|2018-11-15 17:35||geoffclare||Status||Under Review => Resolved|
|2018-11-15 17:35||geoffclare||Resolution||Open => Accepted As Marked|
|2018-11-15 17:35||geoffclare||Tag Attached: tc3-2008|
|2018-11-15 17:37||geoffclare||Note Edited: 0004169|
|Mantis 1.1.6[^] Copyright © 2000 - 2008 Mantis Group|