View Issue Details

Jump to NotesJump to History
IDProjectCategoryView StatusDate SubmittedLast Update
00011441003.1(2008)/Issue 7System Interfacespublic2017-06-14 14:312024-06-11 08:52
ReporterVillemoes Assigned Toajosey  
PrioritynormalSeverityEditorialTypeClarification Requested
Status ClosedResolutionAccepted As Marked 
NameRasmus Villemoes
Organization
User Reference
Sectionfmemopen
Page Number
Line Number
Interp Status---
Final Accepted Text0001144:0004169
Summary0001144: May fmemopen(buf, len, "r") modify the buffer
DescriptionIf 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...
parse_file(f);
fclose(f);
}

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?
Desired ActionClarify 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.
Tagstc3-2008

Relationships

has duplicate 0001152 Closedajosey Effect of modifications to buffer underlying a memory stream 

Activities

shware_systems

2017-06-14 16:41

reporter   bugnote:0003761

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.

Villemoes

2017-06-14 19:35

reporter   bugnote:0003762

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:

fmemopen:
f->orig = buf;
f->buf = malloc(len);
f->len = len;
memcpy(f->buf, buf, len);
memset(buf, 0, len);

fclose:
memcpy(f->orig, f->buf, f->len);


The last sentence was about what one can expect from code such as

char a[3] = "xxx", b[3];
f = fmemopen(a, 3, "r");
fread(&b[0], 1, 1, f);
a[1] = 'y';
fread(&b[1], 1, 1, f);

Is b[1] 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], 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().

shware_systems

2017-06-14 23:06

reporter   bugnote:0003763

Last edited: 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[1] and b[1] after that sequence. I don't see the second example precluded as a result, and might be used in a strinsert() or fillpat() implementation.

Villemoes

2017-06-22 07:23

reporter   bugnote:0003796

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[1]/b[1] example above.

geoffclare

2018-11-15 17:33

manager   bugnote:0004169

Last edited: 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.

Issue History

Date Modified Username Field Change
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 => 0001144: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
2018-12-13 16:35 geoffclare Relationship added has duplicate 0001152
2019-11-05 12:07 geoffclare Status Resolved => Applied
2024-06-11 08:52 agadmin Status Applied => Closed