Re: [ros-dev] [ros-diffs] [gschneider] 42402: asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
5 Aug
2009
5 Aug
'09
2:52 a.m.
Hi,
First of all, are you sure that this code is mature enough to care
about minor details? I would say, "@implemented" has been added by
mistake.
About this commit: I tried to call asctime from glibc-2.8 on Linux,
but tm_year=9 works fine (resulting in 1909). I know, it is not
msvcrt. But I don't see any good reason to not allow years before
1970. Furthermore, I'm sure, this function was once introduced to just
transform a date to human-readable format, and it shouldn't care about
the date. Btw, MSDN says nothing
Another tricky question is: How is the UNIX epoch connected with
Reactos (or Windows)?
About 'asctime': it might be holy, but it's "holey". It doesn't
even
check the month and the day of week to fit the ranges 0..11 and 0..6
correspondingly.
So, please, fix the security problems first, and then revert this commit ;)
2009/8/4 <gschneider(a)svn.reactos.org>rg>:
Author: gschneider
Date: Wed Aug 5 04:06:25 2009
New Revision: 42402
URL: http://svn.reactos.org/svn/reactos?rev=42402&view=rev
Log:
asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
Modified:
trunk/reactos/lib/sdk/crt/time/ctime.c
Modified: trunk/reactos/lib/sdk/crt/time/ctime.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/sdk/crt/time/ctime.c?r…
==============================================================================
--- trunk/reactos/lib/sdk/crt/time/ctime.c [iso-8859-1] (original)
+++ trunk/reactos/lib/sdk/crt/time/ctime.c [iso-8859-1] Wed Aug 5 04:06:25 2009
@@ -1200,14 +1200,23 @@
"Jul", "Aug", "Sep", "Oct", "Nov",
"Dec"
};
static char result[26];
-
- (void) sprintf(result, "%.3s %.3s%3d %02d:%02d:%02d %d\n",
- wday_name[timeptr->tm_wday],
- mon_name[timeptr->tm_mon],
- timeptr->tm_mday, timeptr->tm_hour,
- timeptr->tm_min, timeptr->tm_sec,
- TM_YEAR_BASE + timeptr->tm_year);
- return result;
+ char* res = result;
+
+ /* Check for invalid input time */
+ if (timeptr->tm_year <= 69)
+ {
+ res = NULL;
+ }
+ else
+ {
+ sprintf(res, "%.3s %.3s%3d %02d:%02d:%02d %d\n",
+ wday_name[timeptr->tm_wday],
+ mon_name[timeptr->tm_mon],
+ timeptr->tm_mday, timeptr->tm_hour,
+ timeptr->tm_min, timeptr->tm_sec,
+ TM_YEAR_BASE + timeptr->tm_year);
+ }
+ return res;
}
/*
5 Aug
5 Aug
2:53 a.m.
New subject: [ros-diffs] [gschneider] 42402: asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
I didn't really want to send this e-mail right now, but my eeepc has
strong opinions. ;)
2009/8/4 Alexander Potashev <aspotashev(a)gmail.com>om>:
Hi,
First of all, are you sure that this code is mature enough to care
about minor details? I would say, "@implemented" has been added by
mistake.
About this commit: I tried to call asctime from glibc-2.8 on Linux,
but tm_year=9 works fine (resulting in 1909). I know, it is not
msvcrt. But I don't see any good reason to not allow years before
1970. Furthermore, I'm sure, this function was once introduced to just
transform a date to human-readable format, and it shouldn't care about
the date. Btw, MSDN says nothing
Another tricky question is: How is the UNIX epoch connected with
Reactos (or Windows)?
About 'asctime': it might be holy, but it's "holey". It doesn't
even
check the month and the day of week to fit the ranges 0..11 and 0..6
correspondingly.
So, please, fix the security problems first, and then revert this commit ;)
2009/8/4 <gschneider(a)svn.reactos.org>rg>:
Author: gschneider
Date: Wed Aug 5 04:06:25 2009
New Revision: 42402
URL: http://svn.reactos.org/svn/reactos?rev=42402&view=rev
Log:
asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
Modified:
trunk/reactos/lib/sdk/crt/time/ctime.c
Modified: trunk/reactos/lib/sdk/crt/time/ctime.c
URL:
http://svn.reactos.org/svn/reactos/trunk/reactos/lib/sdk/crt/time/ctime.c?r…
==============================================================================
--- trunk/reactos/lib/sdk/crt/time/ctime.c [iso-8859-1] (original)
+++ trunk/reactos/lib/sdk/crt/time/ctime.c [iso-8859-1] Wed Aug 5 04:06:25 2009
@@ -1200,14 +1200,23 @@
"Jul", "Aug", "Sep", "Oct", "Nov",
"Dec"
};
static char result[26];
-
- (void) sprintf(result, "%.3s %.3s%3d %02d:%02d:%02d %d\n",
- wday_name[timeptr->tm_wday],
- mon_name[timeptr->tm_mon],
- timeptr->tm_mday, timeptr->tm_hour,
- timeptr->tm_min, timeptr->tm_sec,
- TM_YEAR_BASE + timeptr->tm_year);
- return result;
+ char* res = result;
+
+ /* Check for invalid input time */
+ if (timeptr->tm_year <= 69)
+ {
+ res = NULL;
+ }
+ else
+ {
+ sprintf(res, "%.3s %.3s%3d %02d:%02d:%02d %d\n",
+ wday_name[timeptr->tm_wday],
+ mon_name[timeptr->tm_mon],
+ timeptr->tm_mday, timeptr->tm_hour,
+ timeptr->tm_min, timeptr->tm_sec,
+ TM_YEAR_BASE + timeptr->tm_year);
+ }
+ return res;
}
/*
4:21 a.m.
New subject: [ros-diffs] [gschneider] 42402: asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
On Tue, Aug 4, 2009 at 10:52 PM, Alexander Potashev<aspotashev(a)gmail.com> wrote:
First of all, are you sure that this code is mature
enough to care
about minor details? I would say, "@implemented" has been added by
mistake.
The whole @implemented/@unimplemented thing is a mistake. It's really
a worthless measure to go by. I mean lets say function foo takes 20
args but only 10 are useful most of the time and then 8 out of those
are implemented, what do you say? @sorta-buggy-implemented...
I think if we are going to have something like this then we need
autogenerated implementation details on top of the normal API
documentation that we have. My thought is to have something based on
the conformance tests results.
/*
* ShellExecuteEx
*
* Foo Blah Stuff and Things Docu Blah Blah
*
* PARAMS
* Foo gives stuff
*
* Returns
* Bar returns stuff
*
* Implementation Status
*
* %72 implemented
* %10 implementation Fuzz (or failure or something, can't think of
the best term)
* %18 TODO
*/
Some one want to write a bot that does this during release time? I'd
love to be able to hack on it but like all my ideas, I lack the free
time.
--
Steven Edwards
"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo
5:09 a.m.
New subject: [ros-diffs] [gschneider] 42402: asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
Nevermind. I just wanted to say that this function is very broken and
it's a bit strange that you have made another, arguable, change.
So, which test your commit fixes and why is that test correct?
2009/8/5 Steven Edwards <winehacker(a)gmail.com>om>:
On Tue, Aug 4, 2009 at 10:52 PM, Alexander
Potashev<aspotashev(a)gmail.com> wrote:
First of all, are you sure that this code is
mature enough to care
about minor details? I would say, "@implemented" has been added by
mistake.
The whole @implemented/@unimplemented thing is a mistake. It's really
a worthless measure to go by. I mean lets say function foo takes 20
args but only 10 are useful most of the time and then 8 out of those
are implemented, what do you say? @sorta-buggy-implemented...
I think if we are going to have something like this then we need
autogenerated implementation details on top of the normal API
documentation that we have. My thought is to have something based on
the conformance tests results.
/*
* ShellExecuteEx
*
* Foo Blah Stuff and Things Docu Blah Blah
*
* PARAMS
* Foo gives stuff
*
* Returns
* Bar returns stuff
*
* Implementation Status
*
* %72 implemented
* %10 implementation Fuzz (or failure or something, can't think of
the best term)
* %18 TODO
*/
Some one want to write a bot that does this during release time? I'd
love to be able to hack on it but like all my ideas, I lack the free
time.
--
Steven Edwards
"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
5:31 a.m.
New subject: [ros-diffs] [gschneider] 42402: asctime/ctime: Check for too low input time, fixes one msvcrt time winetest
2009/8/5 Alexander Potashev <aspotashev(a)gmail.com>om>:
Nevermind. I just wanted to say that this function is
very broken and
it's a bit strange that you have made another, arguable, change.
I meant
gschneider
So, which test your commit fixes and why is that test correct?
And here too...
2009/8/5 Steven Edwards <winehacker(a)gmail.com>om>:
On Tue, Aug 4, 2009 at 10:52 PM, Alexander
Potashev<aspotashev(a)gmail.com> wrote:
First of all, are you sure that this code is
mature enough to care
about minor details? I would say, "@implemented" has been added by
mistake.
The whole @implemented/@unimplemented thing is a mistake. It's really
a worthless measure to go by. I mean lets say function foo takes 20
args but only 10 are useful most of the time and then 8 out of those
are implemented, what do you say? @sorta-buggy-implemented...
I think if we are going to have something like this then we need
autogenerated implementation details on top of the normal API
documentation that we have. My thought is to have something based on
the conformance tests results.
/*
* ShellExecuteEx
*
* Foo Blah Stuff and Things Docu Blah Blah
*
* PARAMS
* Foo gives stuff
*
* Returns
* Bar returns stuff
*
* Implementation Status
*
* %72 implemented
* %10 implementation Fuzz (or failure or something, can't think of
the best term)
* %18 TODO
*/
Some one want to write a bot that does this during release time? I'd
love to be able to hack on it but like all my ideas, I lack the free
time.
--
Steven Edwards
"There is one thing stronger than all the armies in the world, and
that is an idea whose time has come." - Victor Hugo
_______________________________________________
Ros-dev mailing list
Ros-dev(a)reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev
5580
days inactive
5580
days old
4 comments
2 participants
participants (2)
-
Alexander Potashev -
Steven Edwards