In the core this is not an issue, looking through the codes - any target in a buf[MAXPATHLEN] . Then it's also checked with >=, so there's the room for \0. However yep, the php_sys_readlink is an exported symbol, so when ignoring target_len and a user passed target_len < MAXLENPATH, it'll overflow the target. So I'd rather not touch the places where it's used, but make it respect the target_len and check also dwRet >= target_len || dwRet >= MAXLENPATH ...

Thanks.

You're fully correct. I don't like making assumptions about buffer lengths, wether it is in the core or not. But it's your policy, and I can live with that.

According to its documentation, GetFinalPathNameByHandleA writes up to cchFilePath+1 bytes, which in our case equals to MAXPATHLEN+1: "The size of lpszFilePath, in TCHARs. This value does not include a NULL termination character." But currently there seems to be a bug/misbehaviour/feature in GetFinalPathNameByHandleA, so that cchFilePath includes the terminating null character (have a look at the comments below: https://msdn.microsoft.com/en-us/library/windows/desktop/aa364962%28v=vs.85%29.aspx). The correctness of your code relies on a bug in foreign code. One more assumption :-(

In my eyes, you should fix this and remove the assumptions, as this is cheap and improves your code quality. But you you're right in that this is not really a bug :-(

Kind regards, Jan