Review Request 124413: Enable PAM opening KWallet again

[Lamarque Souza]

> On July 21, 2015, 3:57 p.m., Lamarque Souza wrote:
> > src/runtime/kwalletd/main.cpp, line 113
> > <https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line113>
> >
> >     You should use strncmp instead of strcmp.
> 
> Martin Klapetek wrote:
>     Why would you think? The whole string is being compared, what good would strncmp do in here?

I was thinking about using something like

if (strncmp(argv[x], "--pam-login", sizeof("--pam-login")))

It's a general rule not use use strcmp in security sensitive code since it only stops to compare characters when it finds a null character. If no such character exists in the compared string then you will have a buffer overflow. Since this is an argv string it probably contains a null byte, so the "should" and not "have to". It is just recomendation, you can drop it if you wish.


> On July 21, 2015, 3:57 p.m., Lamarque Souza wrote:
> > src/runtime/kwalletd/main.cpp, line 126
> > <https://git.reviewboard.kde.org/r/124413/diff/1/?file=386596#file386596line126>
> >
> >     You should use strtol() instead of atoi() for better error checking. atoi() does no error checking at all.
> 
> Martin Klapetek wrote:
>     ...but the code does not check for errors (and does not need to)?

Actually, the error checking is done in line 135, so you can drop this one.


- Lamarque


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124413/#review82770
-----------------------------------------------------------


On July 21, 2015, 5:27 p.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124413/
> -----------------------------------------------------------
> 
> (Updated July 21, 2015, 5:27 p.m.)
> 
> 
> Review request for KDE Frameworks, Àlex Fiestas and Valentin Rusu.
> 
> 
> Repository: kwallet
> 
> 
> Description
> -------
> 
> This brings back Alex's patch in commit f2fe3e75b4ba12d0f99aa09327059a1865891b14 [1] which allows KWallet to be opened by PAM if kwallet-pam is present.
> 
> http://quickgit.kde.org/?p=kde-runtime.git&a=commit&h=f2fe3e75b4ba12d0f99aa09327059a1865891b14
> 
> 
> Diffs
> -----
> 
>   src/runtime/kwalletd/main.cpp b4e3837 
> 
> Diff: https://git.reviewboard.kde.org/r/124413/diff/
> 
> 
> Testing
> -------
> 
> Logged in, KWallet does not ask for password anymore.
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150721/9c69a58f/attachment-0001.html>