[ros-dev] Can anybody commit this patch ?

[Alex Ionescu]

Javier Muñoz Mellid wrote:

> Hello Alex,
>
> I have a very bad english so give me a chance if you don't understand 
> all my words :(
>
> This is my first contact with ReactOS development. I am looking in 
> code and learning a lot of things here. I want to contribute and i 
> think that  i am a "acceptable" reverser so i followed a blackbox 
> approach with this patch byte to byte.

I don't have anything against that. Btw, you missed a call to 
SeCaptureSubjectContext in SeCreateAccessState.

>
>> And I would very much appreciate to know
>>
>> 1) Why you had to reverse an opaque structure:
>>    a) It's easy to guess the layout since it was created in NT4 to 
>> manage something new added post NT 3.5.1
>>    b) There's no point in cloning something so opaque that it's not 
>> even in the symbols, since nobody could possibly be using it.
>
>
> You're right but i think that if we get the best match against opaque 
> structures we aren't going to have to change (a lot of) code in the 
> future to adjust "undocumented" drivers by Msoft or 3rd parties. It is 
> my opinion only. For example, i am viewing some drivers incorporating 
> undocumented calls and structures from books like "Undocumented NT" 
> and similar. If we know those structures i think that we can add them. 
> It isn't a design problem and it is only a future's choice.

I totally agree with this as well, this has always been my opinion. But 
the AUX_DATA structure has been specifically opaqued by Microsoft, and 
never published anywhere.

>
>> 2) How you knew that the third member of that structure (or that it 
>> even exists) is an ACCESS_MASK called AccessesToAudit.
>
>
> When i began to reverse SeCreateAccessState it only touch 
> PrivilegedUsed and GenericMapping

Exactly.

> so i get the types and sizes then i mail out my question in our 
> University list at Coruna. I was replied with the structure that i add 
> in the .h It's similar to previous choice. I only need two fields but 
> i was provided with a structure and it has a better match that mine so 
> i add the second (Copy&Paste)

You ended up adding a structure from the Windows Source Code into your 
patch. Perhaps your university has legal access to it through the 
Microsoft Shared Source Code Initiative.

>
>> I've looked at the functions you implemented and it isn't used 
>> anywhere. I've looked with IDA at the binaries, and it's not used 
>> anywhere either.
>
>
> Alex, i read TODO and Security is a beautiful field to me. I grep the 
> unimplemented functions and i found three easy funtions in access.c 
> They were a good choice because they aren't touched for more 
> experienced programmers and so i could implement freely. They look 
> like basic stones to more complicated functions so i can continue 
> adding code in my possibilities.

I know they are easy, I was talking so one of the developers about 
implementing them; that's not the point.

>
> If you want i can attach in this list my SeCreateAccessState's 
> dead-listing from Windows XP no-sp (Spanish version).

No, it's ok.

>
> -Javier
> ______

The problem which I have is that the binary only accesses the first two 
members of that structure. There is no way anyone could've known the 
function of the third member ( I didn't even think/know one existed) 
since it is currently unused, even in Windows Server 2003. Therefore I 
must conclude it was added from internal microsoft headers which were 
emailed to you.

Best regards,
Alex Ionescu