View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0003045 | unreal | ircd | public | 2006-08-30 21:31 | 2006-08-31 05:35 |
| Reporter | satmd | Assigned To | |||
| Priority | normal | Severity | major | Reproducibility | N/A |
| Status | closed | Resolution | wont fix | ||
| Product Version | 3.2.5 | ||||
| Summary | 0003045: match.c: match() does not validate input correctly | ||||
| Description | As discussed on #unreal-support and #unreal-devel: match.c:410, match() gets 2 arguments of type char* which needs validation before being used (e.g. cause modules will call it) The second argument, name is however not properly validated. Line 413 derefernces name, but this will crash with a null pointer exception as name was not checked for NULL. | ||||
| Steps To Reproduce | I did not even try to reproduce after this bug simultaneously crashed 3 of my irc servers at the same code. match() was called with the second argument being NULL by m_hostforward (a patch for this has been suggested, too). I can however not tell why m_hostforward did this, but it IS connected somehow. I had no crashes at all for a long time, but then I had the opportunity to test it against some "victim" :>... One hour later (user trying to evade my bans) the user somehow triggered this behaviour. I wonder if it is a race condition because of calling match() while the user already disconnected - but that's only speculation. | ||||
| Additional Information | Would it be ok to add "&& name" to line 410 (the top "if" clause in there)? transcript: [04:11:15] <aquanight> since you can't really gauruntee that derefing a nullptr will always segv then and there. [04:12:18] <aquanight> imho match() should treat NULL ptr as an empty string [04:13:00] <aquanight> so yes, guu, plz report it :P A fix to m_hostforward was forwarded to Special. | ||||
| 3rd party modules | m_hostforward | ||||
|
|
Well, you're right. The thing is, modules are assumed to be "trusted." We kind of leave it up to the implementor to validate input (something pretty common in C programs). If you send bad input to many C functions, you'll get all sorts of fun stuff ranging from crashes to full blown security holes. I'm not opposed to correcting this, I'm just merely pointing out that there are probably hundreds of other places where we assume the user (i.e. person using the C function) will validate the input before calling the API. |
|
|
[quote]Would it be ok to add "&& name" to line 410 (the top "if" clause in there)?[/quote] If anything it'd be "name &&" - the nullptr test needs to happen before the dereference :P . |
|
|
I agree 100% with codemastr. In fact, in this particular case I don't see ANY reason for a NULL check, furthermore.. I find it ridicilous to add a NULL check in this case, it's the same as adding a NULL check to strcmp. |
| Date Modified | Username | Field | Change |
|---|---|---|---|
| 2006-08-30 21:31 | satmd | New Issue | |
| 2006-08-30 21:31 | satmd | 3rd party modules | => m_hostforward |
| 2006-08-30 23:12 |
|
Note Added: 0012265 | |
| 2006-08-31 00:18 | aquanight | Note Added: 0012266 | |
| 2006-08-31 05:35 | syzop | Status | new => closed |
| 2006-08-31 05:35 | syzop | Note Added: 0012267 | |
| 2006-08-31 05:35 | syzop | Resolution | open => wont fix |
