Skip to content

fix some unsafe behavior in std.format #4427

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
schveiguy merged 1 commit into from
Jun 14, 2016
Merged

fix some unsafe behavior in std.format #4427

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions std/format.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -988,7 +988,7 @@ struct FormatSpec(Char)
const widthOrArgIndex = parse!uint(tmp);
enforceFmt(tmp.length,
text("Incorrect format specifier %", trailing[i .. $]));
i = tmp.ptr - trailing.ptr;
i = arrayPtrDiff(tmp, trailing);
Copy link
Contributor

@ntrel ntrel Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we could use a more general ptrValue safe function:
i = tmp.ptrValue - trailing.ptrValue;
See dlang/druntime#1590 (comment)

Copy link
Member

@schveiguy schveiguy Jun 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ntrel see new PR I added to try and make a nice wrapper for this: dlang/druntime#1592

if (tmp.startsWith('$'))
{
// index of the form %n$
Expand All @@ -1008,7 +1008,7 @@ struct FormatSpec(Char)
{
indexEnd = parse!(typeof(indexEnd))(tmp);
}
i = tmp.ptr - trailing.ptr;
i = arrayPtrDiff(tmp, trailing);
enforceFmt(trailing[i++] == '$',
"$ expected");
}
Expand Down Expand Up @@ -1044,13 +1044,13 @@ struct FormatSpec(Char)
precision = 0;
auto tmp = trailing[i .. $];
parse!int(tmp); // skip digits
i = tmp.ptr - trailing.ptr;
i = arrayPtrDiff(tmp, trailing);
}
else if (isDigit(trailing[i]))
{
auto tmp = trailing[i .. $];
precision = parse!int(tmp);
i = tmp.ptr - trailing.ptr;
i = arrayPtrDiff(tmp, trailing);
}
else
{
Expand Down Expand Up @@ -6549,3 +6549,14 @@ unittest
assert(sformat(buf[], "%s %s %s", "c"c, "w"w, "d"d) == "c w d");
});
}

/*****************************
* The .ptr is unsafe because it could be dereferenced and the length of the array may be 0.
* Returns:
* the difference between the starts of the arrays
*/
@trusted private pure nothrow @nogc
ptrdiff_t arrayPtrDiff(const void[] array1, const void[] array2)
{
return array1.ptr - array2.ptr;
}