View Single Post
Old 26th September 2008, 15:09   #172  |  Link
Oopho2ei
Guest
 
Posts: n/a
A few things which really need to be improved/changed:
Code:
private int trapXorBlock(int pDst, int pSrc, long len) {
   if ( (pDst & 0x3FFFFC) >= MEMSIZE ) return 0x80000001;
   ...
   if (((pDst & 0x3FFFFC) & 0x03) != 0 ) return 0x80000001;
   ...
If only one of the trap parameters is invalid (out of memory bounds) you don't execute the trap and return STATUS_ERROR (see status codes). The "& 0x3FFFFC" fools my parameter checking into 'thinking' that "pDst & 0x3FFFFC == pDst" so don't use it. You can use the mask when executing the trap just to be sure the content code can't break out of your sandbox in case the parameter checking is incomplete. If you are sure the parameter check covers all bad cases you should omit the mask from the entire trap.

The dword,word and byte masks (0x3FFFFC, 0x3FFFFE, 0x3FFFFF) can be derived from the memory size:
Code:
uint32 memsize = 0x400000;
uint32 MASK_DW = (memsize - 1) & 0xFFFFFFFC;
uint32 MASK_W  = (memsize - 1) & 0xFFFFFFFE;
uint32 MASK_B  = (memsize - 1) & 0xFFFFFFFF;
You should replace all occurrences of 0x3FFFFC etc. with the appropriate constant.

Finally please remove the case statement from Trap_AES and use something like:
Code:
if (opOrKeyID < 0xFFF10000)
{
If (opOrKeyID >= player_keys_count) BUG("TRAP_0110: unavailable key accessed") // some self checks (player_keys_count is currently 7)
...
... player_key[opOrKeyID] ...
...
return ... ;
} 

if (opOrKeyID == 0xFFF10000)
{
...
return ... ;
} 

if (opOrKeyID == 0xFFF10001)
{
...
return ... ;
}

// if the below statement is executed there is a bug in the parameter checking
BUG("TRAP_0110: invalid opOrKeyID");
After the parameter check passed opOrKeyID can only be 0,1,2,3,4,5,6,0xFFF10000 or 0xFFF10001. It's not a good programming style to copy/paste the same code 7 times in a row.
If one of the BUG("...") statements is executed the parameter checking is wrong/incomplete.

Thanks for your new release.

Edit: and please only use spaces to format your code (no tabs). My obfuscated aes implementation looks really messed up (another layer of obfuscation added )...

Edit: added some more details to my suggestions

Last edited by Oopho2ei; 26th September 2008 at 16:19.
  Reply With Quote